[PATCH] D108602: [RISCV] Initial support .insn directive for the assembler.

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:880
+    hasNoSchedulingInfo = 1 in {
+  def InsnR : DirectiveInsnR<(outs), (ins uimm7:$opcode, uimm3:$funct3,
+                                          uimm7:$funct7, AnyReg:$rd,
----------------
We don't typically indent inside let blocks


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:917
+// for known formats.
+def : InstAlias<".insn_r $opcode, $funct3, $funct7, $rd, $rs1, $rs2",
+                (InsnR uimm7:$opcode, uimm3:$funct3, uimm7:$funct7, AnyReg:$rd,
----------------
Will writing `.insn_r` in the source get picked up by this and thus expose this detail, or be regarded as an unknown directive and thus give an error?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:923
+                (InsnR4 uimm7:$opcode, uimm3:$funct3, uimm7:$funct7, AnyReg:$rd,
+                       AnyReg:$rs1, AnyReg:$rs2, AnyReg:$rs3), 0>;
+def : InstAlias<".insn_r4 $opcode, $funct3, $funct7, $rd, $rs1, $rs2, $rs3",
----------------
`R4` ones need an extra space


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:564
+let isAllocatable = 0 in
+def AnyReg : RegisterClass<"RISCV", [XLenVT], 32,
+                           (add (sequence "X%u", 0, 31),
----------------
XLenVT is wrong, should probably be untyped or Any or something like that. 32 is bogus too but I see we do that for GPR, presumably it's ignored when you use RegInfoByHwMode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108602