[PATCH] D153748: [RISCV] Add support for XCValu extension in CV32E40P

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td:184
+
+class RVInstAlu_rri<bits<2> funct2, bits<3> funct3, dag outs, dag ins,
+                    string opcodestr, string argstr, list<dag> pattern>
----------------
Can we name all of these CVInst* instead of RVInst* to match some of the earlier classes like `CVBitManipRII`?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td:204
+                   string opcodestr, string argstr, list<dag> pattern>
+    : RVInst<outs, ins, opcodestr, argstr, pattern, InstFormatOther> {
+  bits<5> rs2;
----------------
Can this inherit from RVInstR which will simplify a lot of the body.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td:234
+
+class RVInstAlu_r<bits<7> funct7, bits<3> funct3, dag outs, dag ins,
+                  string opcodestr, string argstr, list<dag> pattern>
----------------
Can this inherit from RVInstR with a `let rs2 = 0b00000`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153748