[PATCH] D121508: [MCA] Moving six instruction flags out of InstrDesc and into InstructionBase.

Fri Mar 11 19:58:39 PST 2022

holland11 created this revision.
holland11 added reviewers: qcolombet, andreadb.
Herald added subscribers: gbedwell, javed.absar, hiraditya.
Herald added a project: All.
holland11 requested review of this revision.
Herald added a project: LLVM.

**TLDR**: This patch proposes moving a few instruction flags out of the InstrDesc and into the InstructionBase. This move allows for those flags to be modified by InstrPostProcess.

This is patch is an audible from https://reviews.llvm.org/D117451 . The primary goal of that/this patch was to give developers the ability (through InstrPostProcess and/or CustomBehaviour) to modify instruction flags such as RetireOOO. The reason why flags like RetireOOO (and the others affected by this patch) can't be modified yet is because they are stored within the InstrDesc object and InstrDesc objects are made const after creation. In my original attempt to solve this problem, I figured that I'd take the opportunity to allow for the other attributes within the InstrDesc to be modified by InstrPostProcess at the same time. This would allow for mca users to be able to do things like modify the instruction's Reads / Defs, MaxLatency, NumMicroOps, and which resources are used. This is something that I wanted to accomplish eventually because currently, CB and IPP have the ability to add *new* hazards to instructions, but they don't have the ability to remove *existing* hazards.

The first problem with my initial proposal was that it was way too "open-ended". The way it let IPP modify the InstrDesc objects gave the user way too much control and not enough guidance. The second problem (and one that contributes to why the first problem is a problem) is that doing things like modifying the MaxLatency attribute, or removing a Use / Def from the Writes / Reads vectors isn't nearly as straight forward as just modifying / removing them. These attributes are originally set with each other in mind and so to modify them correctly can, you may need to modify a set of attributes that all have some form of dependence. For example, if you lower an instruction's MaxLatency, you will likely get an error when running MCA due to MCA thinking the instruction is finished executing, but it still has Reads / Writes active for more cycles.

To solve both of those problems, making the API much more rigid and self explanatory would be desired, if not required. On top of that, I would also need to make sure that I have a full understanding of how all of these attributes are connected together so that I could either design the API in a way that makes it difficult for users to make mistakes or just provide very detailed comments that help users make their modifications in the proper ways.

At this time, I do not feel confident in my ability to do the second part of the above paragraph, but I do still want to be able to modify the flags from this patch (MayLoad, MayStore, HasSideEffects, BeginGroup, EndGroup, and RetireOOO).

There are two main ways that I can achieve this. The first is shown in this diff. What I have done is moved those flags out of the InstrDesc struct and into the InstructionBase class.

The alternative is to leave the flags as part of the InstrDesc struct and inject IPP into the InstrDesc creation function so that it can have a chance to modify the flags. (If done this way, I would have the IPP injection done in a way that would restrict IPP to only modifying these flags.)

The main advantages of doing it in the proposed way are:

- The flags can be modified easily by IPP without actually having to change anything about the way IPP works or is injected.
- The flags can be modified per individual instruction rather than being forced to modify them for every single instruction which matches a particular opcode (only one InstrDesc object is created per opcode so you could have several instructions in the same input that share the same InstrDesc, but they do not share the same InstructionBase).

The main disadvantage is:

- Those flags are now duplicated for every single instruction with the same opcode. So if your input file has 30 instructions all with the same opcode (and you don't want to modify them independently), you'll end up using unnecessary memory from those duplicated / redundant flags.

Since what's being duplicated is only 6 bools, I don't see this as a major disadvantage, but I am absolutely open to changing this patch to conform to the second option (leave the flags in InstrDesc and then inject IPP in a restrictive way to let it modify those flags) if you feel it would be best. I am also open to other ideas, suggestions, questions, and feedback.

This patch also includes a few unrelated and minor changes. I added the opcode to the debug printing for an instruction (so that it's not as much of a hassle to get the opcode number of an in input instruction) and I added the IPP::resetState() function. It's theoretically possible that a target's IPP may maintain some state during its lifetime to help it make decisions based on what it has previously seen in the input. But since IPP is created outside of the CodeRegion loop, IPP should have an opportunity to reset its state whenever a new CodeRegion is being evaluated (since the CodeRegions are supposed to be evaluated independently of each other).

As always, thank you very much for your time. I would appreciate any form of input that anybody might have.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121508

Files:
  llvm/include/llvm/MCA/CustomBehaviour.h
  llvm/include/llvm/MCA/Instruction.h
  llvm/lib/MCA/HardwareUnits/LSUnit.cpp
  llvm/lib/MCA/InstrBuilder.cpp
  llvm/lib/MCA/Stages/DispatchStage.cpp
  llvm/lib/MCA/Stages/ExecuteStage.cpp
  llvm/lib/MCA/Stages/InOrderIssueStage.cpp
  llvm/tools/llvm-mca/Views/SchedulerStatistics.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D121508.414794.patch
Type: text/x-patch
Size: 14305 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220312/3e1c3e20/attachment.bin>