This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Notify ilist traits about in-list transfers
ClosedPublic

Authored by rnk on Jan 23 2019, 2:35 PM.

Details

Summary

Previously no client of ilist traits has needed to know about transfers
of nodes within the same list, so as an optimization, ilist doesn't call
transferNodesFromList in that case. However, now there are clients that
want to use ilist traits to cache instruction ordering information to
optimize dominance queries of instructions in the same basic block.
This change updates the existing ilist traits users to detect in-list
transfers and do nothing in that case.

After this change, we can start caching instruction ordering information
in LLVM IR data structures. There are two main ways to do that:

  • by putting an order integer into the Instruction class
  • by maintaining order integers in a hash table on BasicBlock

I plan to implement and measure both, but I wanted to commit this change
first to enable other out of tree ilist clients to implement this
optimization as well.

Event Timeline

rnk created this revision.Jan 23 2019, 2:35 PM
chandlerc accepted this revision.Jan 23 2019, 2:39 PM

I live in fear of the use cases we haven't yet thought of for this that we will discover in the future. ;] But proceed, LGTM!

Maybe add a unittest exercising this so we don't break it accidentally and fail to notice? No need for another round of review there.

This revision is now accepted and ready to land.Jan 23 2019, 2:39 PM
rnk updated this revision to Diff 183189.Jan 23 2019, 2:56 PM
  • add a unit test
rnk added a comment.Jan 23 2019, 2:57 PM

I live in fear of the use cases we haven't yet thought of for this that we will discover in the future. ;] But proceed, LGTM!

Yeah, this is a pretty intimate specialization point for a data structure to have. But, it's not widely used outside of LLVM's core IR (and MIR) data structures.

Maybe add a unittest exercising this so we don't break it accidentally and fail to notice? No need for another round of review there.

Done, thanks!

This revision was automatically updated to reflect the committed changes.