public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR71104 - call gimplification
@ 2016-05-17 12:28 Richard Biener
  2016-05-18 17:43 ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2016-05-17 12:28 UTC (permalink / raw)
  To: gcc-patches; +Cc: Joseph S. Myers


The following patch addresses PR71104 which shows verify-SSA ICEs
after gimplify-into-SSA.  The issue is that for returns-twice calls
we gimplify register uses in the LHS before the actual call which leads to

  p.0_1 = p;
  _2 = vfork ();
  *p.0_1 = _2;

when gimplifying *p = vfork ().  That of course does not work - 
fortunately the C standard allows to evaluate operands in the LHS
in unspecified order of the RHS.  That also makes this order aligned
with that scary C++ proposal of defined evaluation order.  It also
improves code-generation, avoiding spilling of the pointer load
around the call.

Exchanging the gimplify calls doesn't fix the issue fully as for
aggregate returns we don't gimplify the call result into a
temporary.  So we need to make sure to not emit an SSA when
gimplifying the LHS of a returns-twice call (this path only applies
to non-register returns).

A bootstrap with just the gimplification order exchange is building
target libs right now, I'll re-bootstrap and test the whole thing
again if that succeeds.

Is this ok?  I think it makes sense code-generation-wise.  Code
changes from GCC 6

bar:
.LFB0:
        .cfi_startproc
        subq    $24, %rsp
        .cfi_def_cfa_offset 32
        call    foo
        movq    p(%rip), %rax
        movq    %rax, 8(%rsp)
        call    vfork
        movq    8(%rsp), %rdx
        movl    %eax, (%rdx)
        addq    $24, %rsp
        .cfi_def_cfa_offset 8
        ret

to

bar:
.LFB0:
        .cfi_startproc
        subq    $8, %rsp
        .cfi_def_cfa_offset 16
        call    foo
        call    vfork
        movq    p(%rip), %rdx
        movl    %eax, (%rdx)
        addq    $8, %rsp
        .cfi_def_cfa_offset 8
        ret

Thanks,
Richard.

2016-05-17  Richard Biener  <rguenther@suse.de>

	PR middle-end/71104
	* gimplify.c (gimplify_modify_expr): Gimplify the RHS before
	gimplifying the LHS.  Make sure to gimplify a returning twice
	call LHS without using SSA names.

	* gcc.dg/pr71104-1.c: New testcase.
	* gcc.dg/pr71104-2.c: Likewise.

Index: gcc/gimplify.c
===================================================================
*** gcc/gimplify.c	(revision 236317)
--- gcc/gimplify.c	(working copy)
*************** gimplify_modify_expr (tree *expr_p, gimp
*** 4708,4717 ****
       that is what we must do here.  */
    maybe_with_size_expr (from_p);
  
-   ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue);
-   if (ret == GS_ERROR)
-     return ret;
- 
    /* As a special case, we have to temporarily allow for assignments
       with a CALL_EXPR on the RHS.  Since in GIMPLE a function call is
       a toplevel statement, when gimplifying the GENERIC expression
--- 4708,4713 ----
*************** gimplify_modify_expr (tree *expr_p, gimp
*** 4729,4734 ****
--- 4725,4746 ----
    if (ret == GS_ERROR)
      return ret;
  
+   /* If we gimplified the RHS to a CALL_EXPR and that call may return
+      twice we have to make sure to gimplify into non-SSA as otherwise
+      the abnormal edge added later will make those defs not dominate
+      their uses.
+      ???  Technically this applies only to the registers used in the
+      resulting non-register *TO_P.  */
+   bool saved_into_ssa = gimplify_ctxp->into_ssa;
+   if (saved_into_ssa
+       && TREE_CODE (*from_p) == CALL_EXPR
+       && call_expr_flags (*from_p) & ECF_RETURNS_TWICE)
+     gimplify_ctxp->into_ssa = false;
+   ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue);
+   gimplify_ctxp->into_ssa = saved_into_ssa;
+   if (ret == GS_ERROR)
+     return ret;
+ 
    /* In case of va_arg internal fn wrappped in a WITH_SIZE_EXPR, add the type
       size as argument to the call.  */
    if (TREE_CODE (*from_p) == WITH_SIZE_EXPR)
Index: gcc/testsuite/gcc.dg/pr71104-1.c
===================================================================
*** gcc/testsuite/gcc.dg/pr71104-1.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr71104-1.c	(working copy)
***************
*** 0 ****
--- 1,11 ----
+ /* { dg-do compile } */
+ 
+ void foo(void);
+ int vfork(void);
+ int *p;
+ 
+ void bar(void)
+ {
+   foo();
+   *p = vfork();
+ }
Index: gcc/testsuite/gcc.dg/pr71104-2.c
===================================================================
*** gcc/testsuite/gcc.dg/pr71104-2.c	(revision 0)
--- gcc/testsuite/gcc.dg/pr71104-2.c	(working copy)
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ 
+ struct Foo { char c[1024]; };
+ void foo(void);
+ struct Foo baz(void) __attribute__((returns_twice));
+ struct Foo *p;
+ 
+ void bar(void)
+ {
+   foo();
+   *p = baz();
+ }

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-07-13  7:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-17 12:28 [PATCH] Fix PR71104 - call gimplification Richard Biener
2016-05-18 17:43 ` Jeff Law
2016-07-13  7:59   ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).