| Message ID | OF40F9874B.8CC4E239-ONC22578BF.003E7CE1-C22578BF.00423CAC@il.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 30, 2011 at 2:00 PM, Razya Ladelsky <RAZYA@il.ibm.com> wrote: > Hi, > > This patch fixes the build failure of gcc spec2006 benchmark. > The change is in gimple_duplicate_sese_tail(), where we need to subtract > 1 from the loop's number of iterations. > The stmt created to change the rhs of the loop's condition, used to be > inserted right after the defining stmt of the rhs (an ssa name). > This requires special handling of different cases of the defining stmt: > 1.gimple_stmt > 2.gimple_phi > 3.default_def > > Instead of handling each case separately, if we insert the new stmt at the > begining of the loop's preheader, we're sure that > it's already been defined. > > Bootstrap and testsuite pass successfully (as autopar is not enabled by > default). > No new regressions when the testsuite is run with autopar enabled. > No new regressions for the run of spec2006 with autopar enabled, and gcc > benchmark now passes successfully.. > > OK for trunk? + preheader = loop_preheader_edge(orig_loop)->src; space after loop_preheader_edge. Otherwise ok. Thanks, Richard. > Thanks, > Razya > >
Hi, > This patch fixes the build failure of gcc spec2006 benchmark. > The change is in gimple_duplicate_sese_tail(), where we need to subtract > 1 from the loop's number of iterations. > The stmt created to change the rhs of the loop's condition, used to be > inserted right after the defining stmt of the rhs (an ssa name). > This requires special handling of different cases of the defining stmt: > 1.gimple_stmt > 2.gimple_phi > 3.default_def > > Instead of handling each case separately, if we insert the new stmt at the > begining of the loop's preheader, we're sure that > it's already been defined. > > Bootstrap and testsuite pass successfully (as autopar is not enabled by > default). > No new regressions when the testsuite is run with autopar enabled. > No new regressions for the run of spec2006 with autopar enabled, and gcc > benchmark now passes successfully.. > > OK for trunk? actually, I think a better approach would be not to have this kind of pass-specific adjustments in gimple_duplicate_sese_tail at all. The code decreasing the number of iterations in gimple_duplicate_sese_tail only works because parloops does induction variable canonicalization first; should we need it to be used anywhere else, it will break. I.e., parloops should first transform the condition so that it does the comparison with the adjusted value, and then gimple_duplicate_sese_tail could do just code copying and cfg changes, Zdenek
Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote on 30/06/2011 15:21:43: > From: Zdenek Dvorak <rakdver@kam.mff.cuni.cz> > To: Razya Ladelsky/Haifa/IBM@IBMIL > Cc: gcc-patches@gcc.gnu.org, Richard Guenther <richard.guenther@gmail.com> > Date: 30/06/2011 15:21 > Subject: Re: PATCH] PR 49580 > > Hi, > > > This patch fixes the build failure of gcc spec2006 benchmark. > > The change is in gimple_duplicate_sese_tail(), where we need to subtract > > 1 from the loop's number of iterations. > > The stmt created to change the rhs of the loop's condition, used to be > > inserted right after the defining stmt of the rhs (an ssa name). > > This requires special handling of different cases of the defining stmt: > > 1.gimple_stmt > > 2.gimple_phi > > 3.default_def > > > > Instead of handling each case separately, if we insert the new stmt at the > > begining of the loop's preheader, we're sure that > > it's already been defined. > > > > Bootstrap and testsuite pass successfully (as autopar is not enabled by > > default). > > No new regressions when the testsuite is run with autopar enabled. > > No new regressions for the run of spec2006 with autopar enabled, and gcc > > benchmark now passes successfully.. > > > > OK for trunk? > > actually, I think a better approach would be not to have this kind > of pass-specific > adjustments in gimple_duplicate_sese_tail at all. The code > decreasing the number of > iterations in gimple_duplicate_sese_tail only works because parloops > does induction > variable canonicalization first; should we need it to be used > anywhere else, it will break. > I.e., parloops should first transform the condition so that it does > the comparison with > the adjusted value, and then gimple_duplicate_sese_tail could do > just code copying > and cfg changes, > Ok, working on it. Thanks, Razya > Zdenek
Index: gcc/tree-cfg.c =================================================================== --- gcc/tree-cfg.c (revision 174166) +++ gcc/tree-cfg.c (working copy) @@ -5401,7 +5401,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U gimple cond_stmt; edge sorig, snew; basic_block exit_bb; - basic_block iters_bb; tree new_rhs; gimple_stmt_iterator psi; gimple phi; @@ -5501,11 +5500,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME) { - iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))); - for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); gsi_next (&gsi1)) - if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt))) - break; + basic_block preheader; + preheader = loop_preheader_edge(orig_loop)->src; + gsi1 = gsi_after_labels (preheader); new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true, NULL_TREE,false,GSI_CONTINUE_LINKING); }