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

[Matt Davis via Phabricator via llvm-commits]

mattd added a comment.

In https://reviews.llvm.org/D46367#1087554, @andreadb wrote:

> 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).


Thanks for the feedback, and I see your point.  Eventually, I'd like to eliminate the Index entirely.  I suppose we could consider this change an intermediate step towards the goal.  I was originally aiming at removing the Owner pointers in a few places.  At least, for the cases where the Owner was only needed to perform an index-to-instruction lookup.

> 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.

I think that will work.  Since the RCU will hold a reference to the Instruction, by design it would force the object to remain alive.  Once the RCU finishes using the Instruction, then it will automatically be reclaimed.


https://reviews.llvm.org/D46367