[PATCH] D117733: [RISCV] Update Privileged spec to version-20211203: Support Hypervisor Extention

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:453
+  let Inst{31-25} = funct7;
+  let Inst{24-23} = imm2;
+  let Inst{22-20} = funct3;
----------------
This doesn't make sense to me. Why do we have an operand that's always zero and used to fill in Inst bits? Surely these should be hard-coded to 0 as part of the encoding and GPRMemAtomic (which should probably be renamed to something more general... we already abuse it downstream for non-atomic instructions) used for $rs1 to get the immdiate-less memory register operand parsing?

Though in this case shouldn't it just be a funct5? And using an RVInstR with `let rs2 = funct5` (see the atomics for example)?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:472
+  bits<5> imm5;
+  let Inst{11-7} = imm5;
+}
----------------
Similar to loads, should just be `let rd = 0`, surely, with GPRMemAtomic for rs1?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:756
+
+let EmitPriority = 0 in {
+def : InstAlias<"hlv.b $rd, (${rs1})", (HLV_B GPR:$rd, GPR:$rs1, 0)>;
----------------
You get this all for free with GPRMemAtomic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117733