Skip to content

[RISCV] Don't use RVInstIBase for P-ext plui/pli. NFC #149940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 22, 2025

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Jul 21, 2025

These instructions don't have an rs1 field unlike other instructions that use RVInstIBase.

Rename the classes to not use Unary since we have historically used that for a single register operand.

These instructions don't have an rs1 field unlike other instructions
that use RVInstIBase.

Rename the classes to not use Unary since we have historically used
that for a single register operand.
@llvmbot
Copy link
Member

llvmbot commented Jul 21, 2025

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

Author: Craig Topper (topperc)

Changes

These instructions don't have an rs1 field unlike other instructions that use RVInstIBase.

Rename the classes to not use Unary since we have historically used that for a single register operand.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoP.td (+20-12)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
index 90aa0e0601876..8751558fa1d61 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoP.td
@@ -44,26 +44,34 @@ 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 RVPLoadImm10<bits<7> funct7, string opcodestr,
+                   DAGOperand TyImm10 = simm10>
+    : RVInst<(outs GPR:$rd), (ins TyImm10:$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 RVPLoadImm8<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 +148,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 : RVPLoadImm10<0b1011000, "pli.h">;
 let Predicates = [HasStdExtP, IsRV64] in
-def PLI_W : RVPUnaryImm10<0b1011001, "pli.w">;
+def PLI_W : RVPLoadImm10<0b1011001, "pli.w">;
 let Predicates = [HasStdExtP] in
-def PLI_B : RVPUnaryImm8<0b10110100, "pli.b">;
+def PLI_B : RVPLoadImm8<0b10110100, "pli.b">;
 
 let Predicates = [HasStdExtP] in {
 def PSEXT_H_B : RVPUnaryWUF<0b00, 0b00100, "psext.h.b">;
@@ -157,6 +165,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 : RVPLoadImm10<0b1111000, "plui.h", simm10_unsigned>;
 let Predicates = [HasStdExtP, IsRV64] in
-def PLUI_W : RVPUnaryImm10<0b1111001, "plui.w", simm10_unsigned>;
+def PLUI_W : RVPLoadImm10<0b1111001, "plui.w", simm10_unsigned>;

topperc added a commit to topperc/llvm-project that referenced this pull request Jul 21, 2025
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 llvm#149940
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Co-authored-by: Jim Lin <jim@andestech.com>
@topperc topperc merged commit 8b9e760 into llvm:main Jul 22, 2025
6 of 8 checks passed
@topperc topperc deleted the pr/loadimm branch July 22, 2025 04:44
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
These instructions don't have an rs1 field unlike other instructions
that use RVInstIBase.

Rename the classes to not use Unary since we have historically used that
for a single register operand.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants