On Fri, Aug 30, 2013 at 09:49:59AM -0700, Easwaran Raman wrote:
> 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.

Sure, renumbering the entire BB would be only for compile time reasons;
if you end up with a huge bb where 90% of stmts will have the same UID,
it is as if you weren't using UIDs at all and always walked the entire BB
to find out what stmt precedes what.  You could spend a lot of time in
appears_later_in_bb.

Looking at the code, couple of nits:
  return ((bb_a == bb_b && gimple_uid (a)  < gimple_uid (b))
extra space before <.

  gsi_next (&gsi);
  if (gsi_end_p (gsi))
    return stmt1;
  for (; !gsi_end_p (gsi); gsi_next (&gsi))
    {
...
    }
  return stmt1;

Why not just for (gsi_next (&gsi); !gsi_end_p (gsi); gsi_next (&gsi)) ?
The extra if (gsi_end_p (gsi)) return stmt1; doesn't make any sense
when the loop does exactly that too.

And for the debug stmts, are you sure you are resetting only those debug
stmts which you have to reset?  I mean, if there are say debug stmts using
the SSA_NAME in other bb, it doesn't need to be reset (unless you reuse
the same SSA_NAME for something else), and the compiler even has code
to reconstruct SSA_NAMEs that have been removed, but were still referenced
in debug stmts, using expressions in debug stmts and debug temporaries.

        Jakub

Reply via email to