[PATCH] D151850: [RISCV] Model all 3 arithmetic sources of vector FMA at MC layer.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1271
 // Vector Widening Integer Multiply-Add Instructions
-let Constraints = "@earlyclobber $vd", RVVConstraint = WidenV in {
+let Constraints = "@earlyclobber $vd_wb, $vd = $vd_wb",
+    RVVConstraint = WidenV in {
----------------
Aren't these properties (including `WidenV`) inherent to `VWMAC_MV_V_X` and `VWMAC_MV_X` instructions? I wonder why we're not setting them in the classes themselves.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1377
 // Vector Widening Floating-Point Fused Multiply-Add Instructions
-let Constraints = "@earlyclobber $vd", RVVConstraint = WidenV,
+let Constraints = "@earlyclobber $vd_wb, $vd = $vd_wb", RVVConstraint = WidenV,
     Uses = [FRM], mayRaiseFPException = true in {
----------------
Same question about properties inherent to `VWMAC_FV_V_F`


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvfbf.td:29
+let Predicates = [HasStdExtZvfbfwma],
+    Constraints = "@earlyclobber $vd_wb, $vd = $vd_wb",
     RVVConstraint = WidenV, Uses = [FRM], mayRaiseFPException = true in {
----------------
craig.topper wrote:
> frasercrmck wrote:
> > I wonder if it's worth having `ExtraConstraints` which are automatically appended to `Constraints`, to avoid us having to duplicate the "inherent" tied operand constraints that the base class should be setting.
> > 
> > I don't know how practical that is given our class hierarchies, just a thought. It might make things harder to understand, not easier.
> automatically appended in tablegen itself or somewhere in our class hierarchy?
It wasn't very well fleshed out, sorry. I had something in mind using in our class hierarchy. I think what you've done in your latest update has assuaged most of my concerns. I'll say what I had in mind anyway though.

I was considering there being inside the base class `let Constraints = !strconcat("@earlyclobber $vd_wb", ExtraConstraints)` (which defaults to being empty), and then any defs like this one can do `let ExtraConstraints = ", $vd = $vd_wb"`.

The notion of additive constraints makes sense to me, as I don't think changing or removing such things from the base classes is easy to follow, and repetition is more error-prone. But I think it's very specific to the class hierarchies whether this idea actually makes anything simpler or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151850