Re: RFA: tuples merge for GIMPLE_MODIFY_STMT

[Aldy Hernandez <aldyh at redhat dot com>]

> > +	if (TREE_CODE (lhs) == SSA_NAME)
> > +	  {
> > +	    if (SSA_NAME_DEF_STMT (lhs) == *tp)
> > +	      def_stmt_self_p = true;
> > +	    else
> > +	      debug_tree (*tp);
> >             ^^^^^^^^^^^^^^^^^
> Leftover debugging code?

Fixed.

> > +	/* If we re-gimplify a set to an SSA_NAME, we must change the
> > +	   SSA name's DEF_STMT link.  */
> > +	if (def_stmt_self_p)
> > +	  SSA_NAME_DEF_STMT (GIMPLE_STMT_OPERAND (*tp, 0)) = *tp;
> > +
> >
> But that would mean that the optimizers are generating MODIFY_EXPRs.
> Isn't this papering over something?  We do allow the optimizers to
> emit non-GIMPLE code, but when they emit an assignment, it had better
> be a GIMPLE_MODIFY_STMT.

The problem is that we are creating INIT_EXPR nodes after gimplification in
internal_get_tmp_var(), which end up as MODIFY_EXPRs in statement
annotations.  

I have fixed this in the included patch, by generating a GIMPLE_MODIFY_STMT
instead of INIT_EXPR.  I'm not to gimplifier savvy enough to know if this will
miss out on optimizations, or if I'm side stepping some tenet of the
gimplifier.  This approach fixes the problem, bootstraps, and does not
introduce any regressions.

What do you think?

> > +  /* FIXME tuples: We need to gimplify into GIMPLE_MODIFY_STMT right
> > +     away, so the helper functions below can be made to only handle
> > +     GIMPLE_MODIFY_STMT's, not MODIFY_EXPR as well.  */
> > +
> >
> What's needed to fix this one?  I'd like to address these FIXMEs right
> away.

Working on it.

> > +  if (code == MODIFY_EXPR && cfun && cfun->gimplified)
> > +    {
> > +      /* WTF?  We should be talking GIMPLE_MODIFY_STMT by now.  */
> > +      gcc_unreachable ();
> >
> s/WTF//

Fixed.

> > +  /* ...except perhaps side_effects and volatility.  ?? */
> > +  TREE_SIDE_EFFECTS (t) = side_effects;
> > +  TREE_THIS_VOLATILE (t) = (TREE_CODE_CLASS (code) == tcc_reference
> > +	             	    && arg0 && TREE_THIS_VOLATILE (arg0));
> > +
> >
> arg1 could also be volatile.

I am following prior art on build2_stat which does not take into account
arg1.  I don't think it's wise to have the build2_stat and build2_gimple_stat
behave differently.

Is this patch OK for the branch?  It has been bootstrapped and tested on
x86-64.

Aldy

        * tree.c (build2_stat): Fix comment.
        * gimplify.c (internal_get_tmp_var): Change INIT_EXPR to
        GIMPLE_MODIFY_STMT.
        (tree_to_gimple_tuple): Remove debugging statement.
        Do not re-set the DEF_STMT link.

Index: tree.c
===================================================================
--- tree.c	(revision 118382)
+++ tree.c	(working copy)
@@ -2996,7 +2996,7 @@ build2_stat (enum tree_code code, tree t
 
   if (code == MODIFY_EXPR && cfun && cfun->gimplified)
     {
-      /* WTF?  We should be talking GIMPLE_MODIFY_STMT by now.  */
+      /* We should be talking GIMPLE_MODIFY_STMT by now.  */
       gcc_unreachable ();
     }
 
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 118462)
+++ gimplify.c	(working copy)
@@ -609,7 +609,7 @@ internal_get_tmp_var (tree val, tree *pr
   if (TREE_CODE (TREE_TYPE (t)) == COMPLEX_TYPE)
     DECL_COMPLEX_GIMPLE_REG_P (t) = 1;
 
-  mod = build2 (INIT_EXPR, TREE_TYPE (t), t, unshare_expr (val));
+  mod = build2 (GIMPLE_MODIFY_STMT, TREE_TYPE (t), t, unshare_expr (val));
 
   if (EXPR_HAS_LOCATION (val))
     SET_EXPR_LOCUS (mod, EXPR_LOCUS (val));
@@ -622,7 +622,7 @@ internal_get_tmp_var (tree val, tree *pr
   /* If we're gimplifying into ssa, gimplify_modify_expr will have
      given our temporary an ssa name.  Find and return it.  */
   if (gimplify_ctxp->into_ssa)
-    t = TREE_OPERAND (mod, 0);
+    t = GIMPLE_STMT_OPERAND (mod, 0);
 
   return t;
 }
@@ -3514,8 +3514,6 @@ tree_to_gimple_tuple (tree *tp)
 	  {
 	    if (SSA_NAME_DEF_STMT (lhs) == *tp)
 	      def_stmt_self_p = true;
-	    else
-	      debug_tree (*tp);
 	  }
 
         gs = &make_node (GIMPLE_MODIFY_STMT)->gstmt;
@@ -3529,11 +3527,6 @@ tree_to_gimple_tuple (tree *tp)
         gs->block = TREE_BLOCK (*tp);
         *tp = (tree)gs;
 
-	/* If we re-gimplify a set to an SSA_NAME, we must change the
-	   SSA name's DEF_STMT link.  */
-	if (def_stmt_self_p)
-	  SSA_NAME_DEF_STMT (GIMPLE_STMT_OPERAND (*tp, 0)) = *tp;
-
         return;
       }
     default: