This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR71104 - call gimplification


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();
+ }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]