Hi,

[Honza, for you a question below]

On Wed, 13 Jun 2012, Richard Guenther wrote:

> > Was a non-implemented optimization.  If the compound literal value 
> > isn't used as lvalue and doesn't have its address taken (and generally 
> > fits the current predicate) we can as well subst it in place instead 
> > of going over an intermediate statement.
> >
> > Regstrapping on x86_64-linux in progress.  Okay if that passes?
> 
> Ok.

It doesn't due to a missed folding that nobody else is doing either 
(pr44214-[13].c).  Formerly we had:

   D.1712 = { 2.0e+0, 2.0e+0 };
   *a = *b / D.1712;

which ccp (!) finally folded into

   *a = *b * { 0.5, 0.5 };

Now we start right away with

   *a = *b / { 2.0e+0, 2.0e+0 };

Nothing currently folds this (fold doesn't because it still sees the 
compound literal, and the tree optimizers never change something on that 
statement again, so there's no reason to re-fold).

So, let's finally fold statements in the gimplifier like in the below 
patch.  If you prefer I can conditionalize the folding also on being 
in the generic->gimple lowering, in difference to the other calls into the 
gimplifier from the optimizers.

This exposes some regressions in the testsuite, two of them easily fixed 
in the testcase and the third because we fold before the symtab is ready,
hence this line was crashing:

    if (!from_decl
        ...
        || (symtab_get_node (from_decl)->symbol.in_other_partition))
      return true;

in can_refer_decl_in_current_unit_p.  Honza: I don't understand this 
particular condition.  If we have a from_decl (i.e. the decl we're 
concerned about stems from the initializer of it) and it is in another 
partition, then this says that we can freely refer to the decl itself?  
That doesn't make sense to me.  That decl might for instance also be in 
that other partition and be hidden.  Hence I'm unsure what to return if 
the symtab isn't ready yet.  In the patch below I'm returning false as in 
"don't know".  But I think the current behaviour is wrong?

In any case, this patch is currently in regstrapping on x86-64.  Okay if 
it passes (modulo changes for the above symtab_get_node() issue)?


Ciao,
Michael.
        * gimplify.c (gimplify_modify_expr): Fold generated statements.
        * gimple-fold.c (can_refer_decl_in_current_unit_p): Check
        symtab node existence before access.

testsuite/
        * gcc.dg/debug/dwarf2/inline3.c: Adjust.
        * gcc.dg/tree-ssa/foldstring-1.c: Adjust.

Index: gimplify.c
===================================================================
*** gimplify.c  (revision 188500)
--- gimplify.c  (working copy)
*************** gimplify_modify_expr (tree *expr_p, gimp
*** 4772,4777 ****
--- 4786,4792 ----
    enum gimplify_status ret = GS_UNHANDLED;
    gimple assign;
    location_t loc = EXPR_LOCATION (*expr_p);
+   gimple_stmt_iterator gsi;
  
    gcc_assert (TREE_CODE (*expr_p) == MODIFY_EXPR
              || TREE_CODE (*expr_p) == INIT_EXPR);
*************** gimplify_modify_expr (tree *expr_p, gimp
*** 4912,4919 ****
        gimple_set_location (assign, EXPR_LOCATION (*expr_p));
      }
  
-   gimplify_seq_add_stmt (pre_p, assign);
- 
    if (gimplify_ctxp->into_ssa && is_gimple_reg (*to_p))
      {
        /* If we've somehow already got an SSA_NAME on the LHS, then
--- 4927,4932 ----
*************** gimplify_modify_expr (tree *expr_p, gimp
*** 4923,4928 ****
--- 4936,4945 ----
        gimple_set_lhs (assign, *to_p);
      }
  
+   gimplify_seq_add_stmt (pre_p, assign);
+   gsi = gsi_last (*pre_p);
+   fold_stmt (&gsi);
+ 
    if (want_value)
      {
        *expr_p = TREE_THIS_VOLATILE (*to_p) ? *from_p : unshare_expr (*to_p);
Index: gimple-fold.c
===================================================================
*** gimple-fold.c       (revision 188500)
--- gimple-fold.c       (working copy)
*************** can_refer_decl_in_current_unit_p (tree d
*** 61,79 ****
    struct cgraph_node *node;
    symtab_node snode;
  
!   /* We will later output the initializer, so we can reffer to it.
       So we are concerned only when DECL comes from initializer of
       external var.  */
    if (!from_decl
        || TREE_CODE (from_decl) != VAR_DECL
!       || !DECL_EXTERNAL (from_decl)
!       || (symtab_get_node (from_decl)->symbol.in_other_partition))
      return true;
!   /* We are concerned ony about static/external vars and functions.  */
    if ((!TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
        || (TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL))
      return true;
!   /* Weakrefs have somewhat confusing DECL_EXTERNAL flag set; they are always 
safe.  */
    if (DECL_EXTERNAL (decl)
        && lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)))
      return true;
--- 61,85 ----
    struct cgraph_node *node;
    symtab_node snode;
  
!   /* We will later output the initializer, so we can refer to it.
       So we are concerned only when DECL comes from initializer of
       external var.  */
    if (!from_decl
        || TREE_CODE (from_decl) != VAR_DECL
!       || !DECL_EXTERNAL (from_decl))
      return true;
!   /* We might be called from fold before symtab is ready, in which case
!      we don't know yet.  */
!   if (!(snode = symtab_get_node (from_decl)))
!     return false;
!   if (snode->symbol.in_other_partition)
!     return true;
!   /* We are concerned only about static/external vars and functions.  */
    if ((!TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
        || (TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL))
      return true;
!   /* Weakrefs have somewhat confusing DECL_EXTERNAL flag set; they
!      are always safe.  */
    if (DECL_EXTERNAL (decl)
        && lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)))
      return true;
Index: testsuite/gcc.dg/debug/dwarf2/inline3.c
===================================================================
*** testsuite/gcc.dg/debug/dwarf2/inline3.c     (revision 188500)
--- testsuite/gcc.dg/debug/dwarf2/inline3.c     (working copy)
***************
*** 1,7 ****
  /* Verify that only one DW_AT_const_value is emitted for baz,
     not for baz abstract DIE and again inside of
     DW_TAG_inlined_subroutine.  */
! /* { dg-options "-O2 -g -dA" } */
  /* { dg-do compile } */
  /* { dg-final { scan-assembler-times " DW_AT_const_value" 1 } } */
  
--- 1,7 ----
  /* Verify that only one DW_AT_const_value is emitted for baz,
     not for baz abstract DIE and again inside of
     DW_TAG_inlined_subroutine.  */
! /* { dg-options "-O2 -g -dA -fmerge-all-constants" } */
  /* { dg-do compile } */
  /* { dg-final { scan-assembler-times " DW_AT_const_value" 1 } } */
  
*************** static inline long
*** 11,16 ****
--- 11,19 ----
  foo (void)
  {
    const struct A baz = { .i = 2, .j = 21 };
+   /* We must make sure that baz isn't optimized away before inlining,
+      otherwise its initializer is also lost.  */
+   const struct A *p = &baz;
    asm volatile ("" : : : "memory");
    return baz.i * baz.j;
  }
Index: testsuite/gcc.dg/tree-ssa/foldstring-1.c
===================================================================
*** testsuite/gcc.dg/tree-ssa/foldstring-1.c    (revision 188500)
--- testsuite/gcc.dg/tree-ssa/foldstring-1.c    (working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-O1 -fdump-tree-fre1" } */
  
  void
  arf ()
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-O1 -fdump-tree-gimple" } */
  
  void
  arf ()
*************** arf ()
*** 7,11 ****
    if (""[0] == 0)
      blah ();
  }
! /* { dg-final { scan-tree-dump-times "= 0;" 1 "fre1"} } */
! /* { dg-final { cleanup-tree-dump "fre1" } } */
--- 7,11 ----
    if (""[0] == 0)
      blah ();
  }
! /* { dg-final { scan-tree-dump-times "= 0;" 1 "gimple"} } */
! /* { dg-final { cleanup-tree-dump "gimple" } } */

Reply via email to