| Message ID | Pine.LNX.4.64.1006301543010.18619@wotan.suse.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 30, 2010 at 3:53 PM, Michael Matz <matz@suse.de> wrote: > 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. Yes, that works for me. Richard. > (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));
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));