In that case, if my insert_stmt immediately follows dep_stmt and both have the same UID, not_dominated_by would return true and I will end up updating insert_stmt to dep_stmt which is wrong.
- Easwaran On Fri, May 24, 2013 at 1:07 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Thu, May 23, 2013 at 7:26 PM, Easwaran Raman <era...@google.com> wrote: >> This addresses the case where UID alone is not sufficient to figure >> out which statement appears earlier in a BB. Bootstraps and no test >> regressions in x86_64 on linux. Ok for trunk? > > Why not simply conservatively use gimple_uid (a) <= gimple_uid (b) > in not_dominated_by? > > Richard. > > > >> Thanks, >> Easwaran >> >> >> 2013-05-23 Easwaran Raman <era...@google.com> >> >> PR tree-optimization/57337 >> * tree-ssa-reassoc.c (appears_later_in_bb): New function. >> (find_insert_point): Correctly identify the insertion point >> when two statements with the same UID is compared. >> >> Index: gcc/tree-ssa-reassoc.c >> =================================================================== >> --- gcc/tree-ssa-reassoc.c (revision 199211) >> +++ gcc/tree-ssa-reassoc.c (working copy) >> @@ -2866,6 +2866,31 @@ not_dominated_by (gimple a, gimple b) >> >> } >> >> +/* Among STMT1 and STMT2, return the statement that appears later. Both >> + statements are in same BB and have the same UID. */ >> + >> +static gimple >> +appears_later_in_bb (gimple stmt1, gimple stmt2) >> +{ >> + unsigned uid = gimple_uid (stmt1); >> + gimple_stmt_iterator gsi = gsi_for_stmt (stmt1); >> + gsi_next (&gsi); >> + if (gsi_end_p (gsi)) >> + return stmt1; >> + for (; !gsi_end_p (gsi); gsi_next (&gsi)) >> + { >> + gimple stmt = gsi_stmt (gsi); >> + >> + /* If STMT has a different UID than STMT1 and we haven't seen >> + STMT2 during traversal, we know STMT1 appears later. */ >> + if (gimple_uid (stmt) != uid) >> + return stmt1; >> + else if (stmt == stmt2) >> + return stmt2; >> + } >> + gcc_unreachable (); >> +} >> + >> /* Find the statement after which STMT must be moved so that the >> dependency from DEP_STMT to STMT is maintained. */ >> >> @@ -2875,7 +2900,11 @@ find_insert_point (gimple stmt, gimple dep_stmt) >> gimple insert_stmt = stmt; >> if (dep_stmt == NULL) >> return stmt; >> - if (not_dominated_by (insert_stmt, dep_stmt)) >> + if (gimple_uid (insert_stmt) == gimple_uid (dep_stmt) >> + && gimple_bb (insert_stmt) == gimple_bb (dep_stmt) >> + && insert_stmt != dep_stmt) >> + insert_stmt = appears_later_in_bb (insert_stmt, dep_stmt); >> + else if (not_dominated_by (insert_stmt, dep_stmt)) >> insert_stmt = dep_stmt; >> return insert_stmt; >> }