[RISCV] Support hypervisor extention instructions

[tangxingxin1008]

Summary

According to privileged spec version-20211203

Add the following hypervisor instructions::
        - HLV.B HLV.BU
        - HLV.H HLV.HU HLVX.HU
        - HLV.W HLV.WU HLVX.WU
        - HLV.D
        - HSV.B HSV.H HSV.W HSV.D

Signed-off-by: eric.tang <[e-mail]>

[jrtc27]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

453

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)?

472

Similar to loads, should just be let rd = 0, surely, with GPRMemAtomic for rs1?

756

You get this all for free with GPRMemAtomic

[tangxingxin1008]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

453

Good points. Thank you for your suggestions, I will change it later.

472

Good point.

[tangxingxin1008]

Address jrtc27's comment. Thanks.

[asb]

Thanks for the patch. Alongside the inline comments, two minor suggestions:

• There's no need to split it out now I don't think, but the GPRAtomicMemOp->GPRMemZeroOffset is a change that could have been split out to a separate commit and reviewed separately.
• We don't do enough testing for this in general I don't think, but it would be good to add tests that attempting to use the RV64-only hypervisor instructions on RV32 produces an error.

Once the inline comments are addressed and tests added for using the RV64 instructions on RV32, I think it should be ready to land.

llvm/lib/Target/RISCV/RISCVInstrInfo.td

106

I know this comment is copied, but as you're touching it perhaps reword to "A parse method for (${gpr}) or 0(${gpr}), where the 0 will be silently ignored." I'd drop the "Used for GNU as compatibility" bit as I don't think it's adding much information.

738

For a block of instructions like this, we typically place the : at the some column on every line, meaning the right hand side is aligned (and so easy to scan to check the integer parameters).

746

The general preference is to match the order of instructions in the instruction listing table (table 9.1 in the ratified privileged spec update). This would put the RV64 only instructions in a group after the load+store instructions that are available in both RV32 and RV64.

llvm/test/MC/RISCV/priv-invalid.s

1

If anything, it would be better to add a second RUN line for riscv64 alongside the existing riscv32. You could then have priv-rv64-invalid.s for the RV64 only instructions.

[asb]

@tangxingxin1008: Might you have a chance to look at these review comments soon? I've ended up rebasing D117432 on top of this patch in order to use GPRMemZeroOffset, so that's another reason it would be good to land this. Thanks.

[asb]

In D117733#3301000, @asb wrote:

@tangxingxin1008: Might you have a chance to look at these review comments soon? I've ended up rebasing D117432 on top of this patch in order to use GPRMemZeroOffset, so that's another reason it would be good to land this. Thanks.

Just a friendly ping on this.

[tangxingxin1008]

In D117733#3326097, @asb wrote:

In D117733#3301000, @asb wrote:

@tangxingxin1008: Might you have a chance to look at these review comments soon? I've ended up rebasing D117432 on top of this patch in order to use GPRMemZeroOffset, so that's another reason it would be good to land this. Thanks.

Just a friendly ping on this.

I'm very sorry so late back to you. I will update it as soon as possible.

[tangxingxin1008]

In D117733#3266007, @asb wrote:

Thanks for the patch. Alongside the inline comments, two minor suggestions:

• There's no need to split it out now I don't think, but the GPRAtomicMemOp->GPRMemZeroOffset is a change that could have been split out to a separate commit and reviewed separately.
• We don't do enough testing for this in general I don't think, but it would be good to add tests that attempting to use the RV64-only hypervisor instructions on RV32 produces an error.

Once the inline comments are addressed and tests added for using the RV64 instructions on RV32, I think it should be ready to land.

" There's no need to split it out now I don't think", do you mean the two commits [D117654, D117733] should be to one commit?

[asb]

In D117733#3328033, @tangxingxin1008 wrote:

In D117733#3266007, @asb wrote:

Thanks for the patch. Alongside the inline comments, two minor suggestions:

• There's no need to split it out now I don't think, but the GPRAtomicMemOp->GPRMemZeroOffset is a change that could have been split out to a separate commit and reviewed separately.
• We don't do enough testing for this in general I don't think, but it would be good to add tests that attempting to use the RV64-only hypervisor instructions on RV32 produces an error.

Once the inline comments are addressed and tests added for using the RV64 instructions on RV32, I think it should be ready to land.

" There's no need to split it out now I don't think", do you mean the two commits [D117654, D117733] should be to one commit?

Sorry that was a big ambiguous. I just meant that although splitting out the GPRMemZeroOffset change would be helpful, but not essential. But you've done it in D120017 now anyway, which is helpful as it should unblock my other patch. Thanks!

[tangxingxin1008]

Address asb's comment.

[asb]

Thanks, there's just one piece (see below) that was missed and otherwise this looks great to me.

Could you please add test coverage for the RV64-only instructions on RV32 (i.e. demonstrating attempting to use them produces an error).

[tangxingxin1008]

Address asb's comment. Add more tests for the RV64-only instructions on RV32.

[asb]

Thanks, LGTM.

llvm/lib/Target/RISCV/RISCVInstrInfo.td

 }
 
 def ImmZeroAsmOperand : AsmOperandClass {
   let Name = "ImmZero";
   let RenderMethod = "addImmOperands";
   let DiagnosticType = !strconcat("Invalid", Name);
 }
 
 class SImmAsmOperand<int width, string suffix = "">
     : ImmAsmOperand<"S", width, suffix> {
 }
 
 class UImmAsmOperand<int width, string suffix = "">
     : ImmAsmOperand<"U", width, suffix> {
 }
 
 def FenceArg : AsmOperandClass {
let MCOperandPredicate = [{
     if (STI.getTargetTriple().isArch64Bit())
       return isUInt<6>(Imm);
     return isUInt<5>(Imm);
   }];
   let OperandType = "OPERAND_UIMMLOG2XLEN";
   let OperandNamespace = "RISCVOp";
 }
 
+def immzero : Operand<XLenVT>,
+              ImmLeaf<XLenVT, [{return (Imm == 0);}]> {
+  let ParserMatchClass = ImmZeroAsmOperand;
+}
+
 def uimm2 : Operand<XLenVT> {
   let ParserMatchClass = UImmAsmOperand<2>;
   let DecoderMethod = "decodeUImmOperand<2>";
   let OperandType = "OPERAND_UIMM2";
   let OperandNamespace = "RISCVOp";
 }
 
 def uimm3 : Operand<XLenVT> {
class BranchCC_rri<bits<3> funct3, string opcodestr>
     : RVInstB<funct3, OPC_BRANCH, (outs),
               (ins GPR:$rs1, GPR:$rs2, simm13_lsb0:$imm12),
               opcodestr, "$rs1, $rs2, $imm12">,
       Sched<[WriteJmp, ReadJmp, ReadJmp]> {
   let isBranch = 1;
   let isTerminator = 1;
 }
 
-let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in {
 class Load_ri<bits<3> funct3, string opcodestr>
     : RVInstI<funct3, OPC_LOAD, (outs GPR:$rd), (ins GPR:$rs1, simm12:$imm12),
               opcodestr, "$rd, ${imm12}(${rs1})">;
 
+class HLoad_ri<bits<7> funct7, bits<3> funct3, string opcodestr>
+    : RVInstI<0b100, OPC_SYSTEM, (outs GPR:$rd), (ins GPR:$rs1, immzero:$imm2),
+              opcodestr, "$rd, ${imm2}(${rs1})"> {
+  bits<2> imm2;
+  let Inst{31-25} = funct7;
+  let Inst{24-23} = imm2;
+  let Inst{22-20} = funct3;
+}
+}
+
 // Operands for stores are in the order srcreg, base, offset rather than
 // reflecting the order these fields are specified in the instruction
 // encoding.
-let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
+let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in {
 class Store_rri<bits<3> funct3, string opcodestr>
     : RVInstS<funct3, OPC_STORE, (outs),
               (ins GPR:$rs2, GPR:$rs1, simm12:$imm12),
               opcodestr, "$rs2, ${imm12}(${rs1})">;
 
+class HStore_ri<bits<7> funct7, string opcodestr>
+    : RVInstR<funct7, 0b100, OPC_SYSTEM, (outs),
+              (ins GPR:$rs2, GPR:$rs1, immzero:$imm5),
+               opcodestr, "$rs2, ${imm5}(${rs1})">, Sched<[]> {
+  bits<5> imm5;
+  let Inst{11-7} = imm5;
+}
+}
+
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class ALU_ri<bits<3> funct3, string opcodestr>
     : RVInstI<funct3, OPC_OP_IMM, (outs GPR:$rd), (ins GPR:$rs1, simm12:$imm12),
               opcodestr, "$rd, $rs1, $imm12">,
       Sched<[WriteIALU, ReadIALU]>;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class Shift_ri<bits<5> imm11_7, bits<3> funct3, string opcodestr>
def HINVAL_VVMA : RVInstR<0b0010011, 0b000, OPC_SYSTEM, (outs),
                          (ins GPR:$rs1, GPR:$rs2),
                          "hinval.vvma", "$rs1, $rs2">, Sched<[]>;
 
 def HINVAL_GVMA : RVInstR<0b0110011, 0b000, OPC_SYSTEM, (outs),
                          (ins GPR:$rs1, GPR:$rs2),
                          "hinval.gvma", "$rs1, $rs2">, Sched<[]>;
 }
 
+def HLV_B : HLoad_ri<0b0110000, 0b000, "hlv.b">;
+def HLV_BU : HLoad_ri<0b0110000, 0b001, "hlv.bu">;
+def HLV_H : HLoad_ri<0b0110010, 0b000, "hlv.h">;
+def HLV_HU : HLoad_ri<0b0110010, 0b001, "hlv.hu">;
+def HLVX_HU : HLoad_ri<0b0110010, 0b011, "hlvx.hu">;
+def HLV_W : HLoad_ri<0b0110100, 0b000, "hlv.w">;
+def HLVX_WU : HLoad_ri<0b0110100, 0b011, "hlvx.wu">;
+let Predicates = [IsRV64] in {
+def HLV_WU : HLoad_ri<0b0110100, 0b001, "hlv.wu">;
+def HLV_D : HLoad_ri<0b0110110, 0b000, "hlv.d">;
+}
+
+def HSV_B : HStore_ri<0b0110001, "hsv.b">;
+def HSV_H : HStore_ri<0b0110011, "hsv.h">;
+def HSV_W : HStore_ri<0b0110101, "hsv.w">;
+let Predicates = [IsRV64] in
+def HSV_D : HStore_ri<0b0110111, "hsv.d">;
+
+let EmitPriority = 0 in {
+def : InstAlias<"hlv.b $rd, (${rs1})", (HLV_B GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlv.bu $rd, (${rs1})", (HLV_BU GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlv.h $rd, (${rs1})", (HLV_H GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlv.hu $rd, (${rs1})", (HLV_HU GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlvx.hu $rd, (${rs1})", (HLVX_HU GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlv.w $rd, (${rs1})", (HLV_W GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlv.wu $rd, (${rs1})", (HLV_WU GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlvx.wu $rd, (${rs1})", (HLVX_WU GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hlv.d $rd, (${rs1})", (HLV_D GPR:$rd, GPR:$rs1, 0)>;
+def : InstAlias<"hsv.b $rs2, (${rs1})", (HSV_B GPR:$rs2, GPR:$rs1, 0)>;
+def : InstAlias<"hsv.h $rs2, (${rs1})", (HSV_H GPR:$rs2, GPR:$rs1, 0)>;
+def : InstAlias<"hsv.w $rs2, (${rs1})", (HSV_W GPR:$rs2, GPR:$rs1, 0)>;
+def : InstAlias<"hsv.d $rs2, (${rs1})", (HSV_D GPR:$rs2, GPR:$rs1, 0)>;
+}
+
 //===----------------------------------------------------------------------===//
 // Debug instructions
 //===----------------------------------------------------------------------===//
 
 let isBarrier = 1, isReturn = 1, isTerminator = 1 in {
 def DRET : Priv<"dret", 0b0111101>, Sched<[]> {
   let rd = 0;
   let rs1 = 0;

llvm/lib/Target/RISCV/RISCVInstrInfoC.td

def simm6nonzero : Operand<XLenVT>,
   let MCOperandPredicate = [{
     int64_t Imm;
     if (MCOp.evaluateAsConstantImm(Imm))
       return (Imm != 0) && isInt<6>(Imm);
     return MCOp.isBareSymbolRef();
   }];
 }
 
-def immzero : Operand<XLenVT>,
-              ImmLeaf<XLenVT, [{return (Imm == 0);}]> {
-  let ParserMatchClass = ImmZeroAsmOperand;
-}
-
 def CLUIImmAsmOperand : AsmOperandClass {
   let Name = "CLUIImm";
   let RenderMethod = "addImmOperands";
   let DiagnosticType = !strconcat("Invalid", Name);
 }
 
 
 // c_lui_imm checks the immediate range is in [1, 31] or [0xfffe0, 0xfffff].

llvm/test/MC/RISCV/priv-aliases-valid.s

+# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -show-encoding \
+# RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -show-encoding \
+# RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
+# RUN:     | llvm-objdump -M no-aliases -d - \
+# RUN:     | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 < %s \
+# RUN:     | llvm-objdump -M no-aliases -d - \
+# RUN:     | FileCheck -check-prefix=CHECK-INST %s
+
+# CHECK-INST: hlv.b a0, 0(a1)
+# CHECK: encoding: [0x73,0xc5,0x05,0x60]
+hlv.b   a0, (a1)
+
+# CHECK-INST: hlv.bu a0, 0(a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x60]
+hlv.bu  a0, (a1)
+
+# CHECK-INST: hlv.h a1, 0(a2)
+# CHECK: encoding: [0xf3,0x45,0x06,0x64]
+hlv.h   a1, (a2)
+
+# CHECK-INST: hlv.hu a1, 0(a1)
+# CHECK: encoding: [0xf3,0xc5,0x15,0x64]
+hlv.hu  a1, (a1)
+
+# CHECK-INST: hlvx.hu a1, 0(a2)
+# CHECK: encoding: [0xf3,0x45,0x36,0x64]
+hlvx.hu a1, (a2)
+
+# CHECK-INST: hlv.w a2, 0(a2)
+# CHECK: encoding: [0x73,0x46,0x06,0x68]
+hlv.w   a2, (a2)
+
+# CHECK-INST: hlvx.wu a2, 0(a3)
+# CHECK: encoding: [0x73,0xc6,0x36,0x68]
+hlvx.wu a2, (a3)
+
+# CHECK-INST: hsv.b a0, 0(a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x62]
+hsv.b   a0, (a1)
+
+# CHECK-INST: hsv.h a0, 0(a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x66]
+hsv.h   a0, (a1)
+
+# CHECK-INST: hsv.w a0, 0(a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x6a]
+hsv.w   a0, (a1)

llvm/test/MC/RISCV/priv-invalid.s

-# RUN: not llvm-mc -triple riscv32 < %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple riscv64 < %s 2>&1 | FileCheck %s
 
 mret 0x10 # CHECK: :[[@LINE]]:6: error: invalid operand for instruction
 
 sfence.vma zero, a1, a2 # CHECK: :[[@LINE]]:22: error: invalid operand for instruction
 
 sfence.vma a0, 0x10 # CHECK: :[[@LINE]]:16: error: invalid operand for instruction
 
 sinval.vma zero, a1, a2 # CHECK: :[[@LINE]]:22: error: invalid operand for instruction

 
 hinval.vvma zero, a1, a2 # CHECK: :[[@LINE]]:23: error: invalid operand for instruction
 
 hinval.vvma a0, 0x10 # CHECK: :[[@LINE]]:17: error: invalid operand for instruction
 
 hinval.gvma zero, a1, a2 # CHECK: :[[@LINE]]:23: error: invalid operand for instruction
 
 hinval.gvma a0, 0x10 # CHECK: :[[@LINE]]:17: error: invalid operand for instruction
+
+hlv.b a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hlv.b a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hlv.bu a0, 0x10 # CHECK: :[[@LINE]]:12: error: immediate must be zero
+
+hlv.bu a0, 1(a1) # CHECK: :[[@LINE]]:12: error: immediate must be zero
+
+hlv.h a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hlv.h a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hlv.hu a0, 0x10 # CHECK: :[[@LINE]]:12: error: immediate must be zero
+
+hlv.hu a0, 1(a1) # CHECK: :[[@LINE]]:12: error: immediate must be zero
+
+hlvx.hu a0, 0x10 # CHECK: :[[@LINE]]:13: error: immediate must be zero
+
+hlvx.hu a0, 1(a1) # CHECK: :[[@LINE]]:13: error: immediate must be zero
+
+hlv.w a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hlv.w a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hlv.wu a0, 0x10 # CHECK: :[[@LINE]]:12: error: immediate must be zero
+
+hlv.wu a0, 1(a1) # CHECK: :[[@LINE]]:12: error: immediate must be zero
+
+hlv.d a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hlv.d a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.b a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.b a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.h a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.h a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.w a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.w a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.d a0, 0x10 # CHECK: :[[@LINE]]:11: error: immediate must be zero
+
+hsv.d a0, 1(a1) # CHECK: :[[@LINE]]:11: error: immediate must be zero

llvm/test/MC/RISCV/priv-rv64-valid.s

+# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -show-encoding \
+# RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 < %s \
+# RUN:     | llvm-objdump -M no-aliases -d - \
+# RUN:     | FileCheck -check-prefix=CHECK-INST %s
+
+# CHECK-INST: hlv.wu a0, 0(a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x68]
+hlv.wu   a0, (a1)
+
+# CHECK-INST: hlv.wu a0, 0(a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x68]
+hlv.wu   a0, 0(a1)
+
+# CHECK-INST: hlv.d a0, 0(a1)
+# CHECK: encoding: [0x73,0xc5,0x05,0x6c]
+hlv.d  a0, (a1)
+
+# CHECK-INST: hsv.d a0, 0(a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x6e]
+hsv.d   a0, (a1)

llvm/test/MC/RISCV/priv-valid.s

 
 # CHECK-INST: hinval.gvma zero, zero
 # CHECK: encoding: [0x73,0x00,0x00,0x66]
 hinval.gvma zero, zero
 
 # CHECK-INST: hinval.gvma a0, a1
 # CHECK: encoding: [0x73,0x00,0xb5,0x66]
 hinval.gvma a0, a1
+
+# CHECK-INST: hlv.b a0, 0(a1)
+# CHECK: encoding: [0x73,0xc5,0x05,0x60]
+hlv.b   a0, 0(a1)
+
+# CHECK-INST: hlv.bu a0, 0(a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x60]
+hlv.bu  a0, 0(a1)
+
+# CHECK-INST: hlv.h a1, 0(a2)
+# CHECK: encoding: [0xf3,0x45,0x06,0x64]
+hlv.h   a1, 0(a2)
+
+# CHECK-INST: hlv.hu a1, 0(a1)
+# CHECK: encoding: [0xf3,0xc5,0x15,0x64]
+hlv.hu  a1, 0(a1)
+
+# CHECK-INST: hlvx.hu a1, 0(a2)
+# CHECK: encoding: [0xf3,0x45,0x36,0x64]
+hlvx.hu a1, 0(a2)
+
+# CHECK-INST: hlv.w a2, 0(a2)
+# CHECK: encoding: [0x73,0x46,0x06,0x68]
+hlv.w   a2, 0(a2)
+
+# CHECK-INST: hlvx.wu a2, 0(a3)
+# CHECK: encoding: [0x73,0xc6,0x36,0x68]
+hlvx.wu a2, 0(a3)
+
+# CHECK-INST: hsv.b a0, 0(a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x62]
+hsv.b   a0, 0(a1)
+
+# CHECK-INST: hsv.h a0, 0(a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x66]
+hsv.h   a0, 0(a1)
+
+# CHECK-INST: hsv.w a0, 0(a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x6a]
+hsv.w   a0, 0(a1)