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

Michael Maitland via llvm-commits llvm-commits at lists.llvm.org
Thu May 30 13:13:12 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,
----------------
michaelmaitland 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? 

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

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


More information about the llvm-commits mailing list