[PATCH] D104149: [MCA] Adding the CustomBehaviour class to llvm-mca

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 12 06:38:00 PDT 2021


andreadb added inline comments.


================
Comment at: llvm/lib/MCA/InstrBuilder.cpp:619-638
+  std::unique_ptr<Instruction> NewIS =
+      std::make_unique<Instruction>(D, MCI.getOpcode());
+
+  // Build the list of operands to be used by mca::CustomBehaviour
+  for (int Idx = 0, N = MCI.size(); Idx < N; Idx++) {
+    MCAOperand Op;
+    const MCOperand &MCOp = MCI.getOperand(Idx);
----------------
andreadb wrote:
> Three questions:
> 
> 1. Is it necessary to add MCAOperands to every single instruction?
> 2. Is it possible to do this as a post-processing step, if requested by the simulator?
> 3. Can we limit the number of MCAOperand objects that we store in an mca::Instruction?
> 
> Ideally, this logic should be split from the normal `InstrBuilder::createInstruction` and moved to a separate `InstrBuilder::postProcessInstruction` step. That post-processing step would then be performed only if the so-called "custom behaviour" is requested.
> 
> In order to address points 1. and 3., it may be useful to implement a new "MCA Lowering Context" class, which knows about:
>  a) which instructions require a list of MCAOperand, and
>  b) which operands must be lowered to MCAOperand for the custom hazard check to work.
> 
> The MCA lowering context would know about which instructions/operands require a custom post-processing step, and skip the post-processing step for those instructions that can be safely ignored.
> 
> Essentially, have a context class that filters opcodes and rules out which operands are important to keep as MCAOperand, and which are not important for the custom behaviour class.
> 
> To give you an idea: if we know that the custom behaviour class is only interested in the first operand of a specific load opcode, then we should only create MCAOperand objects for that load, and just for its first operand.
> 
> In most cases (correct me if I am wrong), I suspect that we may not need to store MCAOperands for every single instruction out there. So, blindly lowering every MCOperand into MCAOperand - while it work in general - is a bit extreme in my opinion. Keep in mind that most targets won't implement any custom behaviour for their subtargets, and for those targets, translating to MCAOperand would be unnecessary, and it would simply waste space.
> 
> Essentially, if a custom behaviour is not requested/implemented, then we should ignore this extra post-processing step entirely, and not populate the vector of MCAOperands (this is also why it may be worthy to make the new MCAOperand vector field a std::vector).
> 
> I hope it makes sense.
I forgot to mention that, if ordering of operands is important for the custom behaviour check to work, and you are concerned about having gaps in the MCAOperand sequence, then you could use an extra field to store the `original operand index`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104149



More information about the llvm-commits mailing list