[PATCH] D117654: [RISCV] Update Privileged spec to version-20211203: Support Sinval Extention

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.
Herald added a subscriber: pcwang-thead.

Thanks for the patch. I'm not seeing any errors or major issues in this version, but I've suggested a slight cleanup that would reduce duplication across the different instruction definitions.



================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:683
+                         "sfence.vma", "$rs1, $rs2">, Sched<[]>;
+def SINVAL_VMA : RVInstR<0b0001011, 0b000, OPC_SYSTEM, (outs),
+                         (ins GPR:$rs1, GPR:$rs2),
----------------
If you defined a Priv_rr class, you could use that for sfence.vma, sinval.vma, hfence.{v,g}vma and hinval.{v,g}vma.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:697
+
+def HFENCE_VVMA : RVInstR<0b0010001, 0b000, OPC_SYSTEM, (outs),
+                         (ins GPR:$rs1, GPR:$rs2),
----------------
I think these hfence instructions are technically not part of Sinval, but defined in the hypervisor extension. I understand there's a bit of a circular dependency here, so my suggestion would just be to modify the commit title to something like "Support Sinval extension and hypervisor memory management fence instructions"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117654