New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace llvm::iplist<Instruction> with TaggedList<Instruction> in IRFunction #1766

Open
stoklund opened this Issue Oct 2, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@stoklund
Contributor

stoklund commented Oct 2, 2018

I want to introduce a new linked list data structure, TaggedList. This is an intrusive doubly linked list like llvm::iplist with the following differences:

  • Each node has a uint32_t tag member. These tags are maintained online in a strictly increasing order. This means that the relative position of elements in the list can be determined in (fast) constant time, even while inserting and removing elements from the list.
  • The list has a constant time size() operation, maintained as a member in the TaggedList class. This is needed for the online tag maintenance algorithm.
  • Splicing is a linear time operation because the new elements need to be tagged correctly. This is already the case for our specialization of transferNodesFromList().
  • The tag also makes it possible to detect when an end() iterator is dereferenced or incremented.

I have implemented a prototype of TaggedList based on the Two simplified algorithms for maintaining order in a list paper in order to run benchmarks. List insertions are O(log N) with excellent constant factors.

We already have class LiveIntervalsInstructionNumbering in IROptimizer.cpp. This change makes that class redundant by providing the same functionality while also allowing online instruction insertions.

The online instruction ordering is needed to implement advanced memory allocation and scheduling in backends.

Preemptively Answered Questions:

  • No, we can't extend llvm::iplist with this functionality because a) iplist::size() is linear time, and b) despite using 27 template classes for configuration and specialization, iplist doesn't offer the hooks needed.
  • No, we can't implement this as a hack in IRFunction because a) we need the constant time size() operator, and b) IRFunction::getInstrs() exposes the list implementation, and we can't prevent users from modifying the list and invalidating our side tables.
@stoklund

This comment has been minimized.

Contributor

stoklund commented Oct 2, 2018

The average time to insert an element in a list as measured on my MacBook Pro:

  • 256 nodes: 11 ns.
  • 256 K nodes: 125 ns.
  • 256 M nodes: 1597 ns.

The constant factor steps up around 256 K nodes which corresponds to the size of my last level cache. Each list node is 24 bytes in my benchmark program.

For comparison, I've measured untagged list insertions at ~7 ns. Note that this is a very difficult operation to time, though. The time taken is usually dominated by what else is going on in the program.

@SplitInfinity

This comment has been minimized.

Contributor

SplitInfinity commented Oct 9, 2018

@stoklund

This sounds interesting. Is this a good issue for a beginner to work on?

@stoklund

This comment has been minimized.

Contributor

stoklund commented Oct 9, 2018

Is this a good issue for a beginner to work on?

Yes for a Glow beginner, no for a C++ beginner ;-)

I already have a prototype where I worked out a lot of the implementation details of the retagging algorithm. Maybe I should get that in shape so I can submit it as a WIP PR.

@SplitInfinity

This comment has been minimized.

Contributor

SplitInfinity commented Oct 9, 2018

@stoklund

I can take over if you post whatever you have. Perhaps there is value for me in polishing up your existing code.

stoklund added a commit to stoklund/glow that referenced this issue Oct 10, 2018

[WIP] Begin a TaggedList implementation.
This is a very rough start of a TaggedList implementation for pytorch#1766.

Missing parts:

- Typesafe methods on TaggedList<T>.
- Tests!
- Iterators
- Traits with nodeRemoved / nodeAdded callbacks.

@SplitInfinity SplitInfinity self-assigned this Oct 10, 2018

@stoklund

This comment has been minimized.

Contributor

stoklund commented Nov 16, 2018

@SplitInfinity Are you still working on this?

@SplitInfinity

This comment has been minimized.

Contributor

SplitInfinity commented Nov 19, 2018

@stoklund

No, I'll put it back up for grabs.

@SplitInfinity SplitInfinity removed their assignment Nov 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment