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

[Reid Tatge via Phabricator via llvm-commits]

reidtatge added a comment.

Hi Michael, responses to your questions and comments!



================
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.
+
----------------
michaelmaitland wrote:
> 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]>
> ```
Yes, I'd agree that this is reasonably doable in tablegen doing something like what you proposed.   I think the drawback of this approach is that the nodes in your forwarding network are instructions, rather than functional units, so its a very large graph.   The MDL approach is to model the functional-unit-based graph, then deal with relatively rare exceptions on an instruction-by-instruction basis.  So you can describe a full forwarding network with just a few lines of description.


================
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,
----------------
michaelmaitland wrote:
> 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.
Yeah, I agree you could do that, as you suggested in the earlier comment.


================
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.
+
----------------
michaelmaitland wrote:
> Are you sure that forwarding cannot be modeled for these instructions by adding `ReadAdvance` entries for the input operands?
I should clarify that statement - I meant to say that tdscan can't extract forwarding information for those CPUs.  Certainly you can add ReadAdvance records to any CPU.  But since you have to do it for every single input operand (which usually aren't explicitly modeled at all) and annotate all the SchedWrites, its a lot of work.  Forwarding networks are usually relatively simple, since they're typically based on FUs (of which there are 10's), not instructions (of which there are 1000's). 


================
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.
----------------
michaelmaitland wrote:
> 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.
Cool.  You might try modeling it in the MDL language too.  :-)


================
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.
----------------
michaelmaitland wrote:
> 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.
Agreed.  I should amend that comment to clarify the thought.  


================
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.
+
----------------
michaelmaitland wrote:
> 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?
To answer the first question: (example vector vs scalar forwarding)
Yes, there's an easy way to model this.  Functional unit X can be modeled as a "derived" or "compound" functional unit:
                 func_unit VECTOR:SCALAR X(...);
                 func_unit VECTOR Y();
                 func_unit SCALAR Z();
This declares that X contains both a VECTOR unit and a SCALAR unit, and the forwarding network can specify them separately.   So you could say:
                 forward VECTOR -> VECTOR        and/or
                 forward SCALAR -> SCALAR         and/or
                 forward X -> Y
etc.

To answer the second question about exceptions:
The language currently doesn't have a method for specifying exceptions on an instruction-by-instruction basis.  Our thesis is that this doesn't happen much, and can be done on a functional unit (or sub-functional-unit) basis.  But we could certainly add something to the language without much trouble.

An issue that we don't address today is that its possible that a functional unit might forward just a single output value for instructions, because it only has a single connection to the forwarding network.  I'm not aware of any processors that do this, but we might want to be able to manage that possibility.



================
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. 
+
----------------
michaelmaitland wrote:
> Is there a reason why we need a new language to model the forwarding as between resources instead of between reads and writes?
No, this was just explaining the rationale behind the MDL language decisions.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158790