[llvm-commits] Fwd: Speeding up instruction selection

[Roman Levenstein]

Hi Evan,

 Good point. This code is here due to the historical reasons ;-) I
 introduced it when I was experimenting with different alternatives and
 then forgot to remove it... The commit version will not contain it.

 Any further comments on this patch? Can I commit it or is it too early?

 - Roman

 2008/4/21, Evan Cheng <evan.cheng at apple.com>:

> +  if (left->NodeQueueId && right->NodeQueueId)
 >  +    return (left->NodeQueueId < right->NodeQueueId);
 >  +  return left->NodeNum < right->NodeNum;
 >
 >  Why would NodeQueueId ever be zero? Nodes that are entered into the queue
 > must have this field set, no?
 >
 >  Evan
 >
 >
 >  On Apr 18, 2008, at 4:31 AM, Roman Levenstein wrote:
 >
 >
 > >
 > > Hi Evan,
 > >
 > > 2008/4/2, Evan Cheng <evan.cheng at apple.com>:
 > >
 > > >
 > > > On Apr 2, 2008, at 1:57 AM, Roman Levenstein wrote:
 > > >
 > > >
 > > > > 2008/4/2, Evan Cheng <evan.cheng at apple.com>:
 > > > >
 > > > > >
 > > > > > On Apr 2, 2008, at 12:55 AM, Roman Levenstein wrote:
 > > > > >
 > > > > >
 > > > > > > Hi Evan,
 > > > > > >
 > > > > > > 2008/4/1, Evan Cheng <evan.cheng at apple.com>:
 > > > > > >
 > > > > > > > Please hold off checking it in for a bit. llvm tot is having
 > some
 > > > > > > > problems and I'd like to get to the bottom of it first.
 > > > > > > >
 > > > > > >
 > > > > > > OK.
 > > > > > >
 > > > > > >
 > > > > > > > Also, the tie breaker is less than ideal. I think we need a tie-
 > > > > > > > breaker that is "the SUnit that's added to the queue is
 > preferred".
 > > > > > > > That means it prefers nodes which are closer to the end of
 > block.
 > > > > > > > What
 > > > > > > > do you think?
 > > > > > > >
 > > > > > >
 > > > > > > Do you actually mean "the SUnit that's added to the queue LAST (or
 > > > > > > FIRST) is preferred"? I'll think about it.
 > > > > > >
 > > > > >
 > > > > >
 > > > > > Yep "first".  Basically, if all else being equal, let the node
 > that's
 > > > > > ready first be scheduled first. We can add a order id to SUnit which
 > > > > > gets set when it's pushed into the ready queue. What do you think?
 > > > > >
 > > > >
 > > > > Makes sense. The queue should have a global "current id" counter. Its
 > > > > current value is assigned to each node being inserted into the ready
 > > > > queue and then incremented. When the node is removed from the queue
 > > > > for any reason, its queue order id is reset. It should be rather easy
 > > > > to implement.
 > > > >
 > > >
 > >
 > >
 > > >
 > > > > BTW, do you really want this queue order id in the SUnit or in a
 > > > > separate array indexed by SUnit unique ids?
 > > > >
 > > >
 > > >
 > > > It should be in SUnit since the sort functions don't have access to
 > > > ScheduleDAG members.
 > > >
 > >
 > > Please find and review the attached patch implementing:
 > > - a proper tie-breaker as discussed above.
 > > - and unmodified part for replacing the slow std::priority_queue by
 > > std::set, as it I already did before.
 > >
 > > What do you think?
 > >
 > > -Roman
 > > <ScheduleDAGRRList.patch>
 > >
 >
 >