From patchwork Wed Jun 30 13:53:38 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [rfa] Fix PR44699: bootstrap problem
From: Michael Matz <matz@suse.de>
X-Patchwork-Id: 57413
Message-Id: <Pine.LNX.4.64.1006301543010.18619@wotan.suse.de>
To: Richard Guenther <richard.guenther@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Date: Wed, 30 Jun 2010 15:53:38 +0200 (CEST)

Hi,

On Wed, 30 Jun 2010, Richard Guenther wrote:

> > -      if (laststore)
> > +      if (laststore && is_gimple_reg (lhs))
> > +       {
> > +         gimple_set_vdef (laststore, gimple_vdef (stmt));
> > +         update_stmt (laststore);
> > +         move_ssa_defining_stmt_for_defs (laststore, stmt);
> > +         laststore = NULL;
> > +       }
> 
> I don't think using move_ssa_defining_stmt_for_defs is appropriate
> in this function.

While it is pre-existing confusion in this function you're right.  See 
below for the fix (just replacing move_ssa_defining_stmt_for_defs by 
SSA_NAME_DEF_STMT() assigns is enough).

> It will move both virtual and real operand defs.
> Instead the real defs (well, that for the lhs only) should be moved
> to the last stmt in the sequence and the virtual def to the last store.
> 
> The fn also now looks ugly.

For sure.  Can I make it nice later and fix the bootstrap problem on 
Darwin first?  The freeze starts soon.

(FWIW the improvement you sent is not equivalent, it will attach a VDEF to 
the last insn when non is needed, and there's some confusion with the 
last==laststore test).


Ciao,
Michael.

---


Index: gimple-fold.c
===================================================================
--- gimple-fold.c       (revision 161602)
+++ gimple-fold.c       (working copy)
@@ -1150,7 +1150,14 @@ gimplify_and_update_call_from_tree (gimp
          gsi_insert_before (si_p, last, GSI_NEW_STMT);
          gsi_next (si_p);
        }
-      if (laststore)
+      if (laststore && is_gimple_reg (lhs))
+       {
+         gimple_set_vdef (laststore, gimple_vdef (stmt));
+         update_stmt (laststore);
+         SSA_NAME_DEF_STMT (gimple_vdef (stmt)) = laststore;
+         laststore = NULL;
+       }
+      else if (laststore)
        {
          reaching_vuse = make_ssa_name (gimple_vop (cfun), laststore);
          gimple_set_vdef (laststore, reaching_vuse);
@@ -1158,9 +1165,13 @@ gimplify_and_update_call_from_tree (gimp
          laststore = NULL;
        }
       new_stmt = gimple_build_assign (lhs, tmp);
-      gimple_set_vuse (new_stmt, reaching_vuse);
-      gimple_set_vdef (new_stmt, gimple_vdef (stmt));
-      move_ssa_defining_stmt_for_defs (new_stmt, stmt);
+      if (!is_gimple_reg (tmp))
+       gimple_set_vuse (new_stmt, reaching_vuse);
+      if (!is_gimple_reg (lhs))
+       {
+         gimple_set_vdef (new_stmt, gimple_vdef (stmt));
+         SSA_NAME_DEF_STMT (gimple_vdef (stmt)) = new_stmt;
+       }
     }

   gimple_set_location (new_stmt, gimple_location (stmt));