[PATCH] D46367: [llvm-mca] Avoid exposing index values in the MCA interfaces.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Hi Matt,

what about passing an InstRef (i.e. a pair<index, Instruction>) to all those APIs?
I would not need to add a field to Instruction. The index is a SourceMgr concept. The only reason why those functionalities require it is mainly because they have to notify events, and events are based on the instruction index.

So, you could have that methods like checkRAT/checkXXX/etc. now accept an InstRef instead of a "index, Instruction" or (even worse) "index, InstrDesc". I would avout to change the Event classes if possible (at least for now).

About the instruction ownership issue you mentioned in your last paragraph: I am curious to see if it works the idea of having the RCU as the owner. An instruction exists if it is in-flight in the OoO backend. So, it must be known by the RCU through its entire lifetime. You could do some experiments to see if that idea works.


https://reviews.llvm.org/D46367