[PATCH] D158790: [MDL] First full integration of MDL with LLVM

Wed Aug 30 08:58:17 PDT 2023

michaelmaitland added a comment.

I am super pumped to hear more about MDL on our call later today. The thought that you've given to being able to model complex behavior is exciting!

I've given some comments and asked some questions on the documentation I've read so far. The question I keep asking myself is why we need MDL. It sounds like you've done a really thorough job of understanding what the limitations of the TableGen based scheduling constructs are, and you've presented a set of alternative constructs to address these issues. For example, you point out that forwarding is modeled between instructions and it may be more effective to model it between resources. Why don't we accomplish this using a TableGen approach?

================
Comment at: llvm/docs/Mdl/ForwardingNetworks.md:81
+
+A ReadAdvance resource can optionally specify a set of write resources (ValidWrites) which indicate that the adjustment is associated with a forwarding network.  An empty ValidWrites attribute indicates a late-operand-read.  So tablegen can represent both adjustment types (described above), but it _cannot_ represent a combination of the two for a single instruction.
+
----------------
Could this combination be represented by adjusting the ReadAdvance to accommodate both characteristics? For example, if a Read happens 3 cycles later in the pipeline and the input delivers its input 2 cycles sooner, can't we do this:

```
def : ReadAdvance<MyRead, !add(2, 3), [WriteThatGetsForwarded]>
def : ReadAdvance<MyRead, 3, [ /* All other Writes that do not get forwarded*/]>
```
Maybe it would be annoying to enumerate `All other Writes`, but I think we could modify `ReadAdvance` to support an exclude argument that might look something like this:
```
def : ReadAdvance<MyRead, !add(2, 3), [WriteThatGetsForwarded]>
def : ReadAdvance<MyRead, 3, excludeWrites=[WriteThatGetsForwarded]>
```

================
Comment at: llvm/docs/Mdl/ForwardingNetworks.md:89
+
+1. Only three targets use SchedModel forwarding (AArch64, ARM, and PPC), and only three targets (ARM, PPC, and Hexagon) use Itinerary forwarding,
+2. X86 _only _implements late-reads of operands (and has a few bugs in that logic), and no forwarding,
----------------
I am currently working on a `RISCV` patch that will make model forwarding by using ReadAdvances. I think I am able to represent  forwarded-read and a late-operand-read for the same instruction without any modification to ReadAdvance class.

================
Comment at: llvm/docs/Mdl/ForwardingNetworks.md:92
+3. The SchedModel forwarding implementations are very sparse.  It appears that its generally been used to “cherry-pick” a few “important” cases, and in the few CPUs that model forwarding, most instructions don’t have forwarding information associated with them,
+4.  In the few CPUs that model forwarding, the vast majority of instructions don’t have ReadAdvance entries associated with input operands.  Consequently, forwarding cannot be modeled for these instructions.
+
----------------
Are you sure that forwarding cannot be modeled for these instructions by adding `ReadAdvance` entries for the input operands?

================
Comment at: llvm/docs/Mdl/ForwardingNetworks.md:98
+
+1. Modeling forwarding networks in tablegen is tedious, so existing implementations are sparse, resulting in uneven support for forwarding across instructions.
+2. Thats ok, since latency calculations are simply a non-critical heuristic for general-purpose processors.
----------------
The reason we didn't model any ReadAdvances wasn't because it was tedious, but because we weren't modeling our writes correctly in the first place. Now that we've had some time to do a better job at representing writes, we plan on implementing forwarding networks on the `SiFive7` scheduler model.

================
Comment at: llvm/docs/Mdl/ForwardingNetworks.md:99
+1. Modeling forwarding networks in tablegen is tedious, so existing implementations are sparse, resulting in uneven support for forwarding across instructions.
+2. Thats ok, since latency calculations are simply a non-critical heuristic for general-purpose processors.
+3. Trying to replicate this in the MDL language - in an automatic way (tdscan) - is difficult.
----------------
We have found that modeling latency for vector instructions more accurately can lead to a 10% speedup on some benchmarks. I'm not sure I agree that modeling latency is not critical in this case.

================
Comment at: llvm/docs/Mdl/ForwardingNetworks.md:117
+*   Each instruction (or instruction class) describes its own behavior, and may have different specified behaviors on different functional units.
+*   A forwarding network impacts the behavior of a functional unit, _not_ instructions that run on it.  Therefore it is orthogonal to the specification of instruction behaviors.
+
----------------
I think this is a really nice idea. In hardware, forwarding is often property of functional units, not the instructions that run on those units.

However, it may be the case that functional unit X only forwards vector operands or only forwards scalar operands. Can we capture this behavior if we only discuss resource -> resource?

You say below:
```
A relatively common case would be instruction operands that are _not_ connected to the forwarding network.  We need a reasonable way to model these exceptions. 
```
It's likely we'd use this to model the idea I present above. Can you add an example of how to model exceptions in this documentation?

================
Comment at: llvm/docs/Mdl/ForwardingNetworks.md:206
+
+Since SchedModel descriptions tend to conflate forwarding with instructions that read operands in later pipeline phases, we want to separate these two concerns when we generate an MDL description.  Our fundamental assumption is that forwarding to a functional unit has the same behavior for all instructions that run on that functional unit: the value is delivered to the unit some number of cycles - typically 1 - earlier than if the forwarding path wasn’t implemented. 
+
----------------
Is there a reason why we need a new language to model the forwarding as between resources instead of between reads and writes?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158790