[PATCH] D131930: [mlgo] Add in-development instruction based features for regalloc advisor

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 16 17:03:33 PDT 2022


mtrofin added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SlotIndexes.h:148
 
+    void setEntry(IndexListEntry *Entry) {
+      lie.setPointer(Entry);
----------------
aidengrossman wrote:
> mtrofin wrote:
> > (just noticed this) can you avoid this, because it's just for test, and instead just replace the SlotIndex in your collection?
> This is for instantiation of each SlotIndex within the list. I would like to avoid it if possible too, but in order to setup the test case the SlotIndex needs to have a valid `IndexListEntry*` pointer, and there is no publicly exposed constructor that allows setting that, so I'm calling the default constructor and setting the pointer with this function. So it doesn't seem like there's a super practical way to avoid this. Unless there's some easy way to avoid this that I'm just not seeing?
Ugh... I'd rather the ctor be public - that way, the state management of a `SlotIndex` stays the same (once created, it doesn't change its `lie` - easier to understand / maintain)

or (my initial preference) introduce an abstraction around the slot index stuff. IIRC there isn't that much you actually need from that API.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131930/new/

https://reviews.llvm.org/D131930



More information about the llvm-commits mailing list