[llvm-commits] Speeding up instruction selection

[Roman Levenstein]

Hi Dan, Hi Evan,

2008/4/19, Dan Gohman <gohman at apple.com>:
>
>  On Apr 16, 2008, at 9:19 AM, Roman Levenstein wrote:
>  >
>
> > BTW, Dan, have you had any time to look at the patch introducing
>  > queues instead of heaps in the target specific instruction selectors?
>
>
> Is this the ScheduleDAGRRList patch that changes std::priority_queue
>  to std::set? I know you checked in part of those changes already;
>  could you send me an updated patch with just the outstanding changes?

No. I don't mean the ScheduleDAGRRList.pacth. For this one, I proposed
the remaining patch just with the outstanding patches yesterday (see
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080414/061236.html).
And it is also about using the std::set :-)

The patch for instruction selectors was proposed by me here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080303/059215.html
It is related to the Select() and SelectRoot implementations inside
the target-specific code selectors generated by tablegen.

Regarding my questions about std::set vs std;;multiset in the mentioned patch:
Having looked into it again, I have the impression that std::set could
be used and there is no need for std::multiset. Basically, the this
code:
  void AddToISelQueue(SDOperand N) DISABLE_INLINE {
   int Id = N.Val->getNodeId();
   if (Id != -1 && !isQueued(Id)) {
   ...
}
ensures that only one node with a given NodeId can be inserted into
the ISelQueue.

One possible further improvement, I'm thinking about is the following:
Why do we use a sorted data structure (e.g. sorted vector, set) for
the ISelQueue at all?

1) If we look into the SelectRoot logic, it basically takes the node
with the SMALLEST NodeId from the pending selection queue.
2) More over, it seems to me (please, confirm if it is true!) that the
next node taken for selection in the loop inside the SelectRoot, at
any point in time, has a NodeId that is GREATER than the NodeId of the
last selected node.
3) Even if the selection of the node puts new nodes into the selection
queue, they ALWAYS have NodeIds GREATER than that of this node.
Therefore the property (2) is preserved.

I checked (2) and (3) by adding some debug output to the SelectRoot.
Only one test of all llvm/test test-cases does not fulfill this
assumptions and even there it is not quite obvious if it is a
contradiction to (2) and (3) or a bug.

So, assuming that properties (2) and (3) do hold, the following can be
probably done:
- We iterate over the NodeId numbers from 0 till the DAGSize.
- We check if a Node with a given NodeId (by looking into the
TopOrder???), if isQueued(NodeId) is true, i.e. this is in the queue
- We then check if isSelected(NodeId) is true or false and call
Select() for this node, if it is not selected yet.
- If a new node needs to be inserted into the queue, only
setQueued(NodeId) is called

With these changes, no online sorting would be required at all, which
is a great improvement over the current approach.

What do you think about it?  Is it a correct analysis?

- Roman