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

[Patrick Holland via Phabricator via llvm-commits]

holland11 added a comment.

@andreadb I appreciate all of the suggestions. They all make sense to me and I'll get started on implementing them.

1. Once I've made the changes, would it be better for me to update the diff within this post, or create a new post with the same title/body (but with a new diff)?

2. Your concerns about the MCAOperand list are completely reasonable. I'll get started on making the list optional and only storing the operands that the target's CB requests. What do you think would be the best way to make it 'optional' (with respect to the actual datatype of the MCAOperand list within the mca::Instruction class)? Would making it a std::vector that defaults to a size of 0 be smart or can you think of a better way?

What I'm thinking is that within the CustomBehaviour.h file, there will be another class `InstructionProcessor` (open to better names). This class will have a public method `processInstruction()` (modify may be a better word). The generic class's method won't do anything, but targets who implement their own CB will also implement their own `InstructionProcessor` where they can choose to override the `processInstruction()` method. After the `MCInst` is lowered to `mca::Instruction`, but before we add it to the `LoweredSequence`, we will call `InstructionProcessor::processInstruction()`. This method will take both the `MCInst` and `mca::Instruction` and will make any modifications / additions to the `mca::Instruction`.

This way, for targets that don't have a CB or don't need any modifications to the `mca::Instruction` (such as storing specific operands), the `processInstruction()` method can trivially return without doing anything. And for targets who do want to make modifications to the `mca::Instruction`, they can have a `switch` statement within the `processInstruction()` to only modify the desired instructions.

I mentioned in the original post that CB is currently only designed to handle adding dependencies, but not removing them. As a bonus with this design, we'd get an easy way to remove undesired dependencies (on top of the original intention to only store what we need).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104149