[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:903
+    return createStringError(errc::invalid_argument,
+                             "zcmt is not allowed when c is specified");
+
----------------
craig.topper wrote:
> Extension names should be in single quotes.
> 
> Should we use `'zcmt' and 'c' extensions are incompatible` to match line 863?
My understanding is Zcmt still compatible with C if no D/Zcd is present.

`rv32ic_zcmt` in theory is valid combination according to spec[1].

[1] https://github.com/riscv/riscv-code-size-reduction/blob/main/Zc-specification/Zc.adoc#19-zcmt

> This extension reuses some encodings from c.fsdsp. Therefore it is incompatible with Zcd, which is included when C and D extensions are both present.






================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:354-360
+def FeatureStdExtZcmt
+    : SubtargetFeature<"experimental-zcmt", "HasStdExtZcmt", "true",
+                       "'Zcmt' (table jump instuctions for code-size reduction)", 
+                       [FeatureStdExtZca, FeatureStdExtZicsr]>; 
+def HasStdExtZcmt : Predicate<"Subtarget->hasStdExtZcmt() && !Subtarget->hasStdExtC()">,
+                           AssemblerPredicate<(all_of FeatureStdExtZcmt, (not FeatureStdExtC)),
+                           "'Zcmt' (table jump instuctions for code-size reduction)">;
----------------
I would suggest do not check incompatible stuffs here, let RISCVISAInfo handle that, we already did that for others like `Zfinx` and `F`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863