On Thu, Jul 2, 2015 at 7:34 PM, Sebastian Pop <seb...@gmail.com> wrote: > On Thu, Jul 2, 2015 at 1:17 PM, Tobias Grosser <tob...@grosser.es> wrote: >> On 07/02/2015 06:52 PM, Aditya Kumar wrote: >>> >>> gcc/ChangeLog: >>> >>> 2015-07-02 Aditya Kumar <aditya...@samsung.com> >>> Sebastian Pop <s....@samsung.com> >>> >>> * graphite-sese-to-poly.c (rewrite_cross_bb_scalar_deps): >>> Point iterator to use_stmt. >>> >> >> Hi Aditya, >> >> this patch does not explain what was wrong and why this change is correct. >> Could you possibly add such an explanation. > > One of the code refactorings introducing phi node iterators modified > the semantics of this code as described below ... > >> >> Best, >> Tobias >> >> >>> >>> Bug introduced by patch: >>> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@217787 > > ... here. > If you git log grep for this commit, you would see that this patch > reverts this "typo" introduced in a very large patch. >
How about a testcase or 2 or mentioning if it is covered by existing testcases ? And Aditya you may find it instructive to read this https://gcc.gnu.org/contribute.html#patches regards Ramana > Thanks, > Sebastian > >>> --- >>> gcc/graphite-sese-to-poly.c | 7 +++---- >>> 1 file changed, 3 insertions(+), 4 deletions(-) >>> >>> diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c >>> index 271c499..78f10e4 100644 >>> --- a/gcc/graphite-sese-to-poly.c >>> +++ b/gcc/graphite-sese-to-poly.c >>> @@ -2458,11 +2458,10 @@ rewrite_cross_bb_scalar_deps (scop_p scop, >>> gimple_stmt_iterator *gsi) >>> handle_scalar_deps_crossing_scop_limits (scop, def, stmt); >>> >>> FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, def) >>> - if (gimple_code (use_stmt) == GIMPLE_PHI >>> - && (res = true)) >>> + if (gphi *phi = dyn_cast <gphi *> (use_stmt)) >>> { >>> - gphi_iterator psi = gsi_start_phis (gimple_bb (use_stmt)); >>> - >>> + res = true; >>> + gphi_iterator psi = gsi_for_phi (phi); >>> if (scalar_close_phi_node_p (gsi_stmt (psi))) >>> rewrite_close_phi_out_of_ssa (scop, &psi); >>> else >>> >>