| Message ID | Pine.LNX.4.64.1006301522080.18619@wotan.suse.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 30, 2010 at 09:25, Michael Matz <matz@suse.de> wrote: > * gimple-fold.c (gimplify_and_update_call_from_tree): If LHS is > a gimple reg, attach the original VDEF to the last store in the > sequence. OK if it passes testing. Diego.
On Wed, Jun 30, 2010 at 3:25 PM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Tue, 29 Jun 2010, Richard Guenther wrote: > >> On Tue, Jun 29, 2010 at 5:45 PM, Michael Matz <matz@suse.de> wrote: >> > Hello, >> > >> > Allocating an array of length num_ssa_names, then doing foldings or other >> > transformations that can create new SSA names, and then iterating that >> > array again as if it had num_ssa_names elements is not a good idea. >> > >> > This fixes the testcase. I'm going to regstrap this, once the >> > function_arg_advance brokenness is fixed. Okay, assuming this will work? >> >> Ok. > > While it fixes the testcase (and regstraps on linux) it still breaks on > Darwin. We also need to be careful to not attach VDEFs to the generated > new 'lhs = tmp' assignment if LHS is a gimple reg. Those VDEFs would be > removed fairly quickly breaking the vdef/vuse chains again (that was > pre-existing breakage). > > So I'd like to add this hunk to the patch (whole patch regstrapping again > on x86_64-linux). Okay for trunk? > > > Ciao, > Michael. > -- > * gimple-fold.c (gimplify_and_update_call_from_tree): If LHS is > a gimple reg, attach the original VDEF to the last store in the > sequence. > > 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); > + 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. 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. RIchard. > + 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)); > + move_ssa_defining_stmt_for_defs (new_stmt, 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); + move_ssa_defining_stmt_for_defs (laststore, stmt); + 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)); + move_ssa_defining_stmt_for_defs (new_stmt, stmt); + } } gimple_set_location (new_stmt, gimple_location (stmt));