[PATCH] D116645: [RISCV][llvm] Update CSRs

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

Thanks for the patch - I've left a few comments regarding missing tests and a formatting issue. Then I think this should be good to go.



================
Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:181
 
+//===-------------------------------------------------------------------------//
+// Supervisor Configuration
----------------
Nit: the comment heading used here and elsewhere in this patch doesn't match the rest of the file. I'd either:
1) Modify the existing headings prior to this patch
2) Stick to the style used by existing headings


================
Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:255
+def : SysReg<"htimedelta", 0x605>;
+def : SysReg<"htimedeltah", 0x615>;
+
----------------
I'm wondering if this should be isRV32Only. By my understanding, if you're compiling code to operate under HSXLEN=32 then you're effectively targeting RV32.


================
Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:303
 def : SysReg<"mip", 0x344>;
+def : SysReg<"mtinst", 0x34A>;
+def : SysReg<"mtval2", 0x34B>;
----------------
No tests were added for this one.


================
Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:304
+def : SysReg<"mtinst", 0x34A>;
+def : SysReg<"mtval2", 0x34B>;
+
----------------
No tests were added for this one.


================
Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:310
+
+def : SysReg<"menvcfg", 0x30A>;
+let isRV32Only = 1 in
----------------
No tests were added for this one.


================
Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:313
+def : SysReg<"menvcfgh", 0x31A>;
+def : SysReg<"mseccfg", 0x747>;
+let isRV32Only = 1 in
----------------
No tests were added for this one.


================
Comment at: llvm/lib/Target/RISCV/RISCVSystemOperands.td:361
 def : SysReg<"pmpaddr15", 0x3BF>;
+def : SysReg<"pmpaddr16", 0x3C0>;
+def : SysReg<"pmpaddr17", 0x3C1>;
----------------
It's a bit tedious, but a quick script should generate something appropriate. These CSRs have no tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116645