[PATCH] D154412: [RISCV] Add support for XCVbi extension in CV32E40P

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:566
                           "CORE-V MAC custom opcode table");
+    TRY_TO_DECODE_FEATURE(
+        RISCV::FeatureVendorXCVbi, DecoderTableXCVbi32,
----------------
This seems to be a different line wrap style than all the others. Is this clang-format being weird?


================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:774
+def HasVendorXCVbi : Predicate<"Subtarget->hasVendorXCVbi()">,
+                                AssemblerPredicate<(all_of FeatureVendorXCVbi),
+                                "'XCVbi' (Immediate Branching)">;
----------------
This line is indented 1 space too far


================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:775
+                                AssemblerPredicate<(all_of FeatureVendorXCVbi),
+                                "'XCVbi' (Immediate Branching)">;
+
----------------
I probably should have flagged this in the previous reviews, but shouldn't this string say Core-V in it?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td:14
 let DecoderNamespace = "XCVbitmanip" in {
-  class RVInstBitManipRII<bits<2> funct2, bits<3> funct3, dag outs, dag ins,
+  class CVInstBitManipRII<bits<2> funct2, bits<3> funct3, dag outs, dag ins,
                       string opcodestr, string argstr>
----------------
I just commited this rename once I noticed the inconsistency.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td:184
+
+class RVInstImmBranch<bits<3> funct3, dag outs, dag ins,
+                      string opcodestr, string argstr>
----------------
RV -> CV


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154412