On Fri, Aug 30, 2013 at 9:26 AM, Jakub Jelinek <ja...@redhat.com> wrote: > On Fri, Aug 30, 2013 at 09:13:34AM -0700, Easwaran Raman wrote: >> >> There are more similar failures reported in >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached >> >> the updated patch there. Shall I send that for review? Apart from the >> >> debug statement issue, almost all the bugs are due to dependence >> >> violation because certain newly inserted statements do not have the >> >> right UID. Instead of trying to catch all of them, will it be better >> >> if I check if the stmt has a proper uid (non-zero if it is not the >> >> first stmt) and assign a sensible value at the point where it is used >> >> (find_insert_point and appears_later_in_bb) instead of where the stmt >> >> is created? I think that would be less brittle. >> > >> > In the end all this placement stuff should be reverted and done as >> > post-processing after reassoc is finished. You can remember the >> > inserted stmts that are candidates for moving (just set a pass-local >> > flag on them) and assign UIDs for the whole function after all stmts >> > are inserted. >> >> The problem is we need sane UID values as reassoc is happening - not >> after it is finished. As it process each stmt in reassoc_bb, it might >> generate new stmts which might have to be compared in >> find_insert_point or appears_later_in_bb. > > A new stmt will be created with UID 0 and in case there is stmt movement, > you could zero the UID during movement. Then, you can just special case > UID zero when you are looking at UIDs. As on trunk/4.8 gsi_for_stmt is > O(1), you can easily walk a couple of previous and/or later stmts and look > for the first non-zero UID around it, and treat it as if the UID was > previous non-zero UID + 0.5 or next non-zero UID - 0.5. And, if you see > too many consecutive statements with UID 0 (more than some constant, 32?), > just reassign UIDs to the whole bb. Or you could initially assign UIDs > with some gaps, then keep filling those gaps and once you fill them, > renumber again with new gaps. > Jakub
Yes, this is pretty much what I was proposing. The current implementation doesn't rely on UIDs being unique - they only have to be monotonically non-decreasing. So, when a UID of 0 is encountered, go up till a non-zero UID is found and then go down and assign this non-zero UID. This effectively implies that each UID-0 stmt is visited at most twice. I don't think we need to check if I see more than certain number of UID-0 stmts and redo the entire BB. - Easwaran
