[llvm] [BOLT] Refactor MCInstReference and move it to Core (NFC) (PR #155846)
Anatoly Trosinenko via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 17 08:05:59 PDT 2025
atrosinenko wrote:
@aaupov This is a good question, especially given that one of the constructors accept a pair of basic block and the **index** of the instruction, which can mislead the user that at least inserting an instruction after the referenced one is safe. At the very least I have probably to warn the user in the class comment that this reference is invalidated when instructions are inserted/deleted to the containing basic block (IIRC there should be no such issue in the "no-CFG" case with `std::map` iterators, unless the referenced instruction itself is removed, of course).
On the other hand, when CFG is available I could store a pair of (pointer to basic block, index) instead of (pointer to basic block, pointer to instruction) - by the way, this could be computed in constant time when constructing a reference, as I already rely on the instructions being stored in a vector inside BinaryBasicBlock and there is already an assertion on that. This way, the referenced instruction would still silently change when an instruction is inserted or removed before the originally referenced one, but I wonder if it would be easier to debug - a different instruction would be returned in place of the originally referenced one, but it would be a valid instruction at least, and we can trivially assert on out-of-bounds references (when there are not enough instruction in the BB anymore).
https://github.com/llvm/llvm-project/pull/155846
More information about the llvm-commits
mailing list