[llvm] [MCA] Parameterize variant scheduling classes by explicit variable (PR #92849)

Aiden Grossman via llvm-commits llvm-commits at lists.llvm.org
Thu May 30 14:54:33 PDT 2024


================
@@ -111,8 +112,15 @@ class InstrBuilder {
   /// or null if there isn't any.
   void setInstRecycleCallback(InstRecycleCallback CB) { InstRecycleCB = CB; }
 
+  /// Create an MCA Instruction from a MC Instruction that contains all the
+  /// relevant state MCA needs for modeling. Variant instructions (e.g.,
+  /// register zeroing idioms) each need their own instruction description
+  /// which is uniqued based on InstructionAddress. This can be an actual
+  /// address or something unique per instruction like a loop iteration
+  /// variable, but it must uniquely identify the instruction being passed in.
   Expected<std::unique_ptr<Instruction>>
-  createInstruction(const MCInst &MCI, const SmallVector<Instrument *> &IVec);
+  createInstruction(const MCInst &MCI, const SmallVector<Instrument *> &IVec,
----------------
boomanaiden154 wrote:

> I am confused on how you say keying by the pointer breaks certain use cases but you want the user to pass in InstructionAddress. Before you added a test case, I thought you meant InstructionAddress is the same as the pointer to the MCInst. Now, it looks like you are passing some arbitrary identifier, and that is confusing to me: 0 and 1 don't represent instruction addresses to me. Is InstructionAddress a bad name for the values you are passing? Did you mean to pass something other than 0 or 1 for this variable in the test case?

The idea is that `InstructionAddress` is unique to each unique `MCInst`. I intended this to be something like an instruction pointer, but something like something guaranteed to be unique like a loop iterator would work too. You're right, the naming isn't too clear. If we end up using it, I'll come up with something better.

> One suggestion I have is to add a unique identifier field to mca::Instruction. It can be a static field that gets incremented by one every time a MCInst is created. Then we can use that as the key, and you don't have to rely on a client of this method to figure out what to pass for InstructionAddress.

Wouldn't that just be equivalent to creating a new variant description for each `MCInst`? We can structure it like that, but that seems to defeat the point of the cache.

https://github.com/llvm/llvm-project/pull/92849


More information about the llvm-commits mailing list