[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

463

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

479

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

759

You get this all for free with GPRMemAtomic

[tangxingxin1008]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

463

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

479

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

108

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.

741

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

749

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);
 }
 
 // A parse method for (${gpr}) or 0(${gpr}), where the 0 is be silently ignored.
 def ZeroOffsetMemOpOperand : AsmOperandClass {
   let Name = "ZeroOffsetMemOpOperand";
   let RenderMethod = "addRegOperands";
   let PredicateMethod = "isGPR";
   let ParserMethod = "parseZeroOffsetMemOp";
 }
 
 def GPRMemZeroOffset : RegisterOperand<GPR> {
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_r<bits<7> funct7, bits<5> funct5, string opcodestr>
+    : RVInstR<funct7, 0b100, OPC_SYSTEM, (outs GPR:$rd),
+              (ins GPRMemZeroOffset:$rs1), opcodestr, "$rd, $rs1"> {
+  let rs2 = funct5;
+}
+}
+
 // 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_rr<bits<7> funct7, string opcodestr>
+    : RVInstR<funct7, 0b100, OPC_SYSTEM, (outs),
+              (ins GPR:$rs2, GPRMemZeroOffset:$rs1),
+               opcodestr, "$rs2, $rs1"> {
+  let rd = 0;
+}
+}
+
 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 SFENCE_VMA  : Priv_rr<"sfence.vma", 0b0001001>, Sched<[]>;
 def SINVAL_VMA  : Priv_rr<"sinval.vma", 0b0001011>, Sched<[]>;
 def HFENCE_VVMA : Priv_rr<"hfence.vvma", 0b0010001>, Sched<[]>;
 def HFENCE_GVMA : Priv_rr<"hfence.gvma", 0b0110001>, Sched<[]>;
 def HINVAL_VVMA : Priv_rr<"hinval.vvma", 0b0010011>, Sched<[]>;
 def HINVAL_GVMA : Priv_rr<"hinval.gvma", 0b0110011>, Sched<[]>;
 
+def HLV_B   : HLoad_r<0b0110000, 0b00000, "hlv.b">, Sched<[]>;
+def HLV_BU  : HLoad_r<0b0110000, 0b00001, "hlv.bu">, Sched<[]>;
+def HLV_H   : HLoad_r<0b0110010, 0b00000, "hlv.h">, Sched<[]>;
+def HLV_HU  : HLoad_r<0b0110010, 0b00001, "hlv.hu">, Sched<[]>;
+def HLVX_HU : HLoad_r<0b0110010, 0b00011, "hlvx.hu">, Sched<[]>;
+def HLV_W   : HLoad_r<0b0110100, 0b00000, "hlv.w">, Sched<[]>;
+def HLVX_WU : HLoad_r<0b0110100, 0b00011, "hlvx.wu">, Sched<[]>;
+def HSV_B   : HStore_rr<0b0110001, "hsv.b">, Sched<[]>;
+def HSV_H   : HStore_rr<0b0110011, "hsv.h">, Sched<[]>;
+def HSV_W   : HStore_rr<0b0110101, "hsv.w">, Sched<[]>;
+
+let Predicates = [IsRV64] in {
+def HLV_WU  : HLoad_r<0b0110100, 0b00001, "hlv.wu">, Sched<[]>;
+def HLV_D   : HLoad_r<0b0110110, 0b00000, "hlv.d">, Sched<[]>;
+def HSV_D   : HStore_rr<0b0110111, "hsv.d">, Sched<[]>;
+}
+
 //===----------------------------------------------------------------------===//
 // Debug instructions
 //===----------------------------------------------------------------------===//
 
 let isBarrier = 1, isReturn = 1, isTerminator = 1 in {
 def DRET : Priv<"dret", 0b0111101>, Sched<[]> {
   let rd = 0;
   let rs1 = 0;
   let rs2 = 0b10010;
 }

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, (a1)
+# CHECK: encoding: [0x73,0xc5,0x05,0x60]
+hlv.b   a0, 0(a1)
+
+# CHECK-INST: hlv.bu a0, (a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x60]
+hlv.bu  a0, 0(a1)
+
+# CHECK-INST: hlv.h a1, (a2)
+# CHECK: encoding: [0xf3,0x45,0x06,0x64]
+hlv.h   a1, 0(a2)
+
+# CHECK-INST: hlv.hu a1, (a1)
+# CHECK: encoding: [0xf3,0xc5,0x15,0x64]
+hlv.hu  a1, 0(a1)
+
+# CHECK-INST: hlvx.hu a1, (a2)
+# CHECK: encoding: [0xf3,0x45,0x36,0x64]
+hlvx.hu a1, 0(a2)
+
+# CHECK-INST: hlv.w a2, (a2)
+# CHECK: encoding: [0x73,0x46,0x06,0x68]
+hlv.w   a2, 0(a2)
+
+# CHECK-INST: hlvx.wu a2, (a3)
+# CHECK: encoding: [0x73,0xc6,0x36,0x68]
+hlvx.wu a2, 0(a3)
+
+# CHECK-INST: hsv.b a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x62]
+hsv.b   a0, 0(a1)
+
+# CHECK-INST: hsv.h a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x66]
+hsv.h   a0, 0(a1)
+
+# CHECK-INST: hsv.w a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x6a]
+hsv.w   a0, 0(a1)

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

 # RUN: not llvm-mc -triple riscv32 < %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]]:16: error: expected '(' after optional integer offset
+
+hlv.b a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hlv.b a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0
+
+hlv.bu a0, 0x10 # CHECK: :[[@LINE]]:17: error: expected '(' after optional integer offset
+
+hlv.bu a0, a1 # CHECK: :[[@LINE]]:12: error: expected '(' or optional integer offset
+
+hlv.bu a0, 1(a1) # CHECK: :[[@LINE]]:12: error: optional integer offset must be 0
+
+hlv.h a0, 0x10 # CHECK: :[[@LINE]]:16: error: expected '(' after optional integer offset
+
+hlv.h a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hlv.h a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0
+
+hlv.hu a0, 0x10 # CHECK: :[[@LINE]]:17: error: expected '(' after optional integer offset
+
+hlv.hu a0, a1 # CHECK: :[[@LINE]]:12: error: expected '(' or optional integer offset
+
+hlv.hu a0, 1(a1) # CHECK: :[[@LINE]]:12: error: optional integer offset must be 0
+
+hlvx.hu a0, 0x10 # CHECK: :[[@LINE]]:18: error: expected '(' after optional integer offset
+    
+hlvx.hu a0, a1 # CHECK: :[[@LINE]]:13: error: expected '(' or optional integer offset
+    
+hlvx.hu a0, 1(a1) # CHECK: :[[@LINE]]:13: error: optional integer offset must be 0
+
+hlv.w a0, 0x10 # CHECK: :[[@LINE]]:16: error: expected '(' after optional integer offset
+
+hlv.w a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hlv.w a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0
+
+hlvx.wu a0, 0x10 # CHECK: :[[@LINE]]:18: error: expected '(' after optional integer offset
+
+hlvx.wu a0, a1 # CHECK: :[[@LINE]]:13: error: expected '(' or optional integer offset
+
+hlvx.wu a0, 1(a1) # CHECK: :[[@LINE]]:13: error: optional integer offset must be 0
+
+hsv.b a0, 0x10 # CHECK: :[[@LINE]]:16: error: expected '(' after optional integer offset
+
+hsv.b a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hsv.b a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0
+
+hsv.h a0, 0x10 # CHECK: :[[@LINE]]:16: error: expected '(' after optional integer offset
+
+hsv.h a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hsv.h a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0
+
+hsv.w a0, 0x10 # CHECK: :[[@LINE]]:16: error: expected '(' after optional integer offset
+
+hsv.w a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hsv.w a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0

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

+# RUN: not llvm-mc -triple riscv64 < %s 2>&1 | FileCheck %s
+
+hlv.wu a0, 0x10 # CHECK: :[[@LINE]]:17: error: expected '(' after optional integer offset
+
+hlv.wu a0, a1 # CHECK: :[[@LINE]]:12: error: expected '(' or optional integer offset
+
+hlv.wu a0, 1(a1) # CHECK: :[[@LINE]]:12: error: optional integer offset must be 0
+
+hlv.d a0, 0x10 # CHECK: :[[@LINE]]:16: error: expected '(' after optional integer offset
+
+hlv.d a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hlv.d a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0
+
+hsv.d a0, 0x10 # CHECK: :[[@LINE]]:16: error: expected '(' after optional integer offset
+
+hsv.d a0, a1 # CHECK: :[[@LINE]]:11: error: expected '(' or optional integer offset
+
+hsv.d a0, 1(a1) # CHECK: :[[@LINE]]:11: error: optional integer offset must be 0

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
+
+# RUN: not llvm-mc -triple riscv32 < %s 2>&1 \
+# RUN:     | FileCheck -check-prefix=CHECK-RV32 %s
+
+# CHECK-INST: hlv.wu a0, (a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x68]
+# CHECK-RV32: :[[@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
+hlv.wu   a0, (a1)
+
+# CHECK-INST: hlv.wu a0, (a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x68]
+# CHECK-RV32: :[[@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
+hlv.wu   a0, 0(a1)
+
+# CHECK-INST: hlv.d a0, (a1)
+# CHECK: encoding: [0x73,0xc5,0x05,0x6c]
+# CHECK-RV32: :[[@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
+hlv.d  a0, (a1)
+
+# CHECK-INST: hlv.d a0, (a1)
+# CHECK: encoding: [0x73,0xc5,0x05,0x6c]
+# CHECK-RV32: :[[@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
+hlv.d  a0, 0(a1)
+
+# CHECK-INST: hsv.d a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x6e]
+# CHECK-RV32: :[[@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
+hsv.d   a0, (a1)
+
+# CHECK-INST: hsv.d a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x6e]
+# CHECK-RV32: :[[@LINE+1]]:1: error: instruction requires the following: RV64I Base Instruction Set
+hsv.d   a0, 0(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, (a1)
+# CHECK: encoding: [0x73,0xc5,0x05,0x60]
+hlv.b   a0, (a1)
+
+# CHECK-INST: hlv.bu a0, (a1)
+# CHECK: encoding: [0x73,0xc5,0x15,0x60]
+hlv.bu  a0, (a1)
+
+# CHECK-INST: hlv.h a1, (a2)
+# CHECK: encoding: [0xf3,0x45,0x06,0x64]
+hlv.h   a1, (a2)
+
+# CHECK-INST: hlv.hu a1, (a1)
+# CHECK: encoding: [0xf3,0xc5,0x15,0x64]
+hlv.hu  a1, (a1)
+
+# CHECK-INST: hlvx.hu a1, (a2)
+# CHECK: encoding: [0xf3,0x45,0x36,0x64]
+hlvx.hu a1, (a2)
+
+# CHECK-INST: hlv.w a2, (a2)
+# CHECK: encoding: [0x73,0x46,0x06,0x68]
+hlv.w   a2, (a2)
+
+# CHECK-INST: hlvx.wu a2, (a3)
+# CHECK: encoding: [0x73,0xc6,0x36,0x68]
+hlvx.wu a2, (a3)
+
+# CHECK-INST: hsv.b a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x62]
+hsv.b   a0, (a1)
+
+# CHECK-INST: hsv.h a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x66]
+hsv.h   a0, (a1)
+
+# CHECK-INST: hsv.w a0, (a1)
+# CHECK: encoding: [0x73,0xc0,0xa5,0x6a]
+hsv.w   a0, (a1)