[RISCV] Correct the immediate swizzling for P-ext plui.h/w.

[topperc]

If I'm reading the spec correctly, plui.h/w encode the immediate
differently from pli.h/w. pli.h/w appear to rotate the immediate
left by 1 before encoding while plui.h/w rotate the immediate right
by 1 before encoding.

Since I was splitting the classes, I made the name closer to the
instruction names since the immediate width was ambiguous. I've
added an _i suffix to make it similar to base and Zb* class names.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-mc

Author: Craig Topper (topperc)

Changes

If I'm reading the spec correctly, plui.h/w encode the immediate
differently from pli.h/w. pli.h/w appear to rotate the immediate
left by 1 before encoding while plui.h/w rotate the immediate right
by 1 before encoding.

Since I was splitting the classes, I made the name closer to the
instruction names since the immediate width was ambiguous. I've
added an _i suffix to make it similar to base and Zb* class names.

Stacked on #149940

---

Full diff: https://github.com/llvm/llvm-project/pull/149945.diff

3 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVInstrInfoP.td (+34-12)
• (modified) llvm/test/MC/RISCV/rv32p-valid.s (+2-2)
• (modified) llvm/test/MC/RISCV/rv64p-valid.s (+3-3)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
index 90aa0e0601876..a5bb1245e53a4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
@@ -44,26 +44,48 @@ def simm10_unsigned : RISCVOp {
 //===----------------------------------------------------------------------===//
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
-class RVPUnaryImm10<bits<7> funct7, string opcodestr,
-                    DAGOperand TyImm10 = simm10>
-    : RVInstIBase<0b010, OPC_OP_IMM_32, (outs GPR:$rd), (ins TyImm10:$imm10),
-                  opcodestr, "$rd, $imm10"> {
+class PLI_i<bits<7> funct7, string opcodestr>
+    : RVInst<(outs GPR:$rd), (ins simm10:$imm10), opcodestr, "$rd, $imm10",
+             [], InstFormatOther> {
   bits<10> imm10;
+  bits<5> rd;
 
   let Inst{31-25} = funct7;
   let Inst{24-16} = imm10{8-0};
   let Inst{15}    = imm10{9};
+  let Inst{14-12} = 0b010;
+  let Inst{11-7} = rd;
+  let Inst{6-0} = OPC_OP_IMM_32.Value;
 }
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
-class RVPUnaryImm8<bits<8> funct8, string opcodestr>
-    : RVInstIBase<0b010, OPC_OP_IMM_32, (outs GPR:$rd), (ins uimm8:$uimm8),
-                  opcodestr, "$rd, $uimm8"> {
+class PLUI_i<bits<7> funct7, string opcodestr>
+    : RVInst<(outs GPR:$rd), (ins simm10_unsigned:$imm10), opcodestr,
+             "$rd, $imm10", [], InstFormatOther> {
+  bits<10> imm10;
+  bits<5> rd;
+
+  let Inst{31-25} = funct7;
+  let Inst{24}    = imm10{0};
+  let Inst{23-15} = imm10{9-1};
+  let Inst{14-12} = 0b010;
+  let Inst{11-7} = rd;
+  let Inst{6-0} = OPC_OP_IMM_32.Value;
+}
+
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
+class PLI_B_i<bits<8> funct8, string opcodestr>
+    : RVInst<(outs GPR:$rd), (ins uimm8:$uimm8), opcodestr, "$rd, $uimm8", [],
+             InstFormatOther> {
   bits<8> uimm8;
+  bits<5> rd;
 
   let Inst{31-24} = funct8;
   let Inst{23-16} = uimm8;
   let Inst{15}    = 0b0;
+  let Inst{14-12} = 0b010;
+  let Inst{11-7} = rd;
+  let Inst{6-0} = OPC_OP_IMM_32.Value;
 }
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
@@ -140,11 +162,11 @@ def PSSLAI_W : RVPUnaryImm5<0b101, "psslai.w">;
 } // Predicates = [HasStdExtP, IsRV64]
 
 let Predicates = [HasStdExtP] in
-def PLI_H : RVPUnaryImm10<0b1011000, "pli.h">;
+def PLI_H : PLI_i<0b1011000, "pli.h">;
 let Predicates = [HasStdExtP, IsRV64] in
-def PLI_W : RVPUnaryImm10<0b1011001, "pli.w">;
+def PLI_W : PLI_i<0b1011001, "pli.w">;
 let Predicates = [HasStdExtP] in
-def PLI_B : RVPUnaryImm8<0b10110100, "pli.b">;
+def PLI_B : PLI_B_i<0b10110100, "pli.b">;
 
 let Predicates = [HasStdExtP] in {
 def PSEXT_H_B : RVPUnaryWUF<0b00, 0b00100, "psext.h.b">;
@@ -157,6 +179,6 @@ def PSEXT_W_H      : RVPUnaryWUF<0b01, 0b00101, "psext.w.h">;
 } // Predicates = [HasStdExtP, IsRV64]
 
 let Predicates = [HasStdExtP] in
-def PLUI_H : RVPUnaryImm10<0b1111000, "plui.h", simm10_unsigned>;
+def PLUI_H : PLUI_i<0b1111000, "plui.h">;
 let Predicates = [HasStdExtP, IsRV64] in
-def PLUI_W : RVPUnaryImm10<0b1111001, "plui.w", simm10_unsigned>;
+def PLUI_W : PLUI_i<0b1111001, "plui.w">;
diff --git a/llvm/test/MC/RISCV/rv32p-valid.s b/llvm/test/MC/RISCV/rv32p-valid.s
index c259c142f92b2..ffff0f25642a3 100644
--- a/llvm/test/MC/RISCV/rv32p-valid.s
+++ b/llvm/test/MC/RISCV/rv32p-valid.s
@@ -71,8 +71,8 @@ psabs.h a1, a2
 # CHECK-ASM: encoding: [0x9b,0x22,0x73,0xe4]
 psabs.b t0, t1
 # CHECK-ASM-AND-OBJ: plui.h gp, 32
-# CHECK-ASM: encoding: [0x9b,0x21,0x20,0xf0]
+# CHECK-ASM: encoding: [0x9b,0x21,0x08,0xf0]
 plui.h gp, 32
 # CHECK-ASM-AND-OBJ: plui.h gp, -412
-# CHECK-ASM: encoding: [0x9b,0xa1,0x64,0xf0]
+# CHECK-ASM: encoding: [0x9b,0x21,0x99,0xf0]
 plui.h gp, 612
diff --git a/llvm/test/MC/RISCV/rv64p-valid.s b/llvm/test/MC/RISCV/rv64p-valid.s
index 3ea6b00bbe11c..a0d6eadfb6c30 100644
--- a/llvm/test/MC/RISCV/rv64p-valid.s
+++ b/llvm/test/MC/RISCV/rv64p-valid.s
@@ -95,13 +95,13 @@ psabs.h t1, t5
 # CHECK-ASM: encoding: [0x1b,0x25,0x79,0xe4]
 psabs.b a0, s2
 # CHECK-ASM-AND-OBJ: plui.h s2, 4
-# CHECK-ASM: encoding: [0x1b,0x29,0x04,0xf0]
+# CHECK-ASM: encoding: [0x1b,0x29,0x01,0xf0]
 plui.h s2, 4
 # CHECK-ASM-AND-OBJ: plui.h gp, -412
-# CHECK-ASM: encoding: [0x9b,0xa1,0x64,0xf0]
+# CHECK-ASM: encoding: [0x9b,0x21,0x99,0xf0]
 plui.h gp, 612
 # CHECK-ASM-AND-OBJ: plui.w a2, 1
-# CHECK-ASM: encoding: [0x1b,0x26,0x01,0xf2]
+# CHECK-ASM: encoding: [0x1b,0x26,0x00,0xf3]
 plui.w a2, 1
 # CHECK-ASM-AND-OBJ: plui.w a2, -1
 # CHECK-ASM: encoding: [0x1b,0xa6,0xff,0xf3]

[lenary]

I think something went wrong with your merge commit, the end-to-end changes are now just a rewrap of some tablegen.

[topperc]

I think something went wrong with your merge commit, the end-to-end changes are now just a rewrap of some tablegen.

"Files changed" shows 3 files changed including 2 tests.

[lenary]

Sorry, github wasn't showing me that despite me asking it to. :(

[topperc]

spec is here RVP-instrEncodings-015.pdf

PLI.H and PLI.W show imm[8:0|9] in bits 24:15.

PLUI.H shows imm[6|15:7] in bits 24:15. imm is shown as having 16 bits even though it is a 10 bit encoding. I believe this is representing the 16-bit value that will be written to each 16-bit element of the register.

PLUI.W shows imm[22|31:23] in bits 24:15. imm is shown as having 32 bits even though it is a 10 bit encoding. I believe this is representing the 32-bit value that will be written to each 32-bit element of the register.

[lenary]

LGTM. This matches my reading, but all the docs are fairly information-thin on how the assembly corresponds to what is encoded.