[PATCH] D153159: [RISCV] Add errors for mixing Zcmp with C/Zcd and D.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:900
 
-  if (HasZcmt && HasD && HasC)
+  if ((HasZcmt || Exts.count("zcmp")) && Exts.count("d") &&
+      (HasC || Exts.count("zcd")))
----------------
craig.topper wrote:
> VincentWu wrote:
> > VincentWu wrote:
> > > I found there might be a problem about predicate of `Zcd`
> > > 
> > > Zcd contains: `c.fld, c.fldsp, c.fsd, c.fsdsp`, witch is a subset of D ext.
> > > it directly depends on Zca instead of C ext.
> > > 
> > > Thus, `c.fld, c.fldsp, c.fsd, c.fsdsp` are valid when `HasZcd || (HasD && HasC)`
> > > 
> > > Zcmp is directly incompatible with Zcd, so there should be two error msg
> > > 
> > > `'Zcmp' extension is incompatible with 'Zcd'` 
> > > and `'Zcmp' extension is incompatible with both 'C' and 'D' are enabled`
> > same problem also happens on MC layer.
> > 
> > I am trying to fix it by 
> > ```
> > def HasCompressedDoubleFloat
> >     : Predicate<"Subtarget->hasStdExtZcd() || (Subtarget->hasStdExtC() && Subtarget->hasStdExtC())">,
> >                 AssemblerPredicate<(any_of FeatureStdExtZcd, (all_of FeatureStdExtD, FeatureStdExtC)),
> >                                    "'Zcd' (Compressed Double-Precision Floating-Point Instructions) or "
> >                                    "both 'C' (Compressed Instructions) and "
> >                                    "'D' (Double-Precision Floating-Point)">;
> > ```
> > 
> > but it seems `AssemblerPredicate` can't process nested conditions 
> I don't think the Zcd instructions should be enabled without D. That's not a very useful behavior. There wouldn't be much point in making CPU support Zcd without D.
I think Zcd might imply D and Zcf on RV32 might imply F. That was stated by Krste here https://lists.riscv.org/g/tech-code-size/message/1544

```
I think even the normative text can state Zcf requires F and Zcd
requires D (which means it also requires F).  It's OK for normative
text to be redundant at times, and I think this would reduce
confusion.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153159