[PATCH] D89449: [RISCV] Initial infrastructure for code generation of the RISC-V V-extension

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/CMakeLists.txt:16
 tablegen(LLVM RISCVGenSystemOperands.inc -gen-searchable-tables)
+tablegen(LLVM RISCVGenSearchableTables.inc -gen-searchable-tables)
 
----------------
I think these are in alphabetical order?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoPseudoV.td:174
+{
+  foreach instruction = [!cast<Instruction>(instruction_name#"_VV_"# vlmul.MX)] in
+  def : Pat<(result_type (vop
----------------
Is this foreach just to make instruction a variable?


================
Comment at: llvm/lib/Target/RISCV/RISCVMCInstLower.cpp:161
+
+  for (const MachineOperand &MO : MI->operands()) {
+    int OpNo = (int)MI->getOperandNo(&MO);
----------------
I think you can use explicit_operands() here to avoid the isImplicit check later.


================
Comment at: llvm/lib/Target/RISCV/RISCVMCInstLower.cpp:173
+    default:
+      report_fatal_error("unknown operand type");
+    case MachineOperand::MO_Register: {
----------------
Can this just be an llvm_unreachable?


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:369
 
-def VRM2 : RegisterClass<"RISCV", [nxv16i8, nxv8i16, nxv4i32, nxv2i64], 64,
-                         (add V26M2, V28M2, V30M2, V8M2, V10M2, V12M2, V14M2, V16M2,
-                              V18M2, V20M2, V22M2, V24M2, V0M2, V2M2, V4M2, V6M2)> {
-  let Size = 128;
-}
+defset list<VReg> AllVRegs = {
 
----------------
Is this list used?


================
Comment at: llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp:4
 #include "llvm/ADT/Triple.h"
+#include "llvm/IR/Intrinsics.h"
 #include "llvm/Support/raw_ostream.h"
----------------
This is arguably a layering violation since Utils is supposed to be usable by the MC layer. And MC layer tools don't use IR. But since its only a header it might not be an issue. Though it might break a modules build?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D89449