Skip to content
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

[RISCV] Rename rs1_wb to rd in some C instructions. NFC #112269

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Oct 14, 2024

The spec refers to the field as rd'/rs1' so we might as well name the destination rd.

The spec refers to the field as rd'/rs1' so we might as
well name the destination rd.
@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2024

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

Author: Craig Topper (topperc)

Changes

The spec refers to the field as rd'/rs1' so we might as well name the destination rd.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+8-8)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index 7d742322b42969..d6ba275921b5ed 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -279,9 +279,9 @@ class Bcz<bits<3> funct3, string OpcodeStr,
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class Shift_right<bits<2> funct2, string OpcodeStr, RegisterClass cls,
                   Operand ImmOpnd>
-    : RVInst16CB<0b100, 0b01, (outs cls:$rs1_wb), (ins cls:$rs1, ImmOpnd:$imm),
+    : RVInst16CB<0b100, 0b01, (outs cls:$rd), (ins cls:$rs1, ImmOpnd:$imm),
                  OpcodeStr, "$rs1, $imm"> {
-  let Constraints = "$rs1 = $rs1_wb";
+  let Constraints = "$rs1 = $rd";
   let Inst{12} = imm{5};
   let Inst{11-10} = funct2;
   let Inst{6-2} = imm{4-0};
@@ -477,10 +477,10 @@ def C_SRAI : Shift_right<0b01, "c.srai", GPRC, uimmlog2xlennonzero>,
              Sched<[WriteShiftImm, ReadShiftImm]>;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
-def C_ANDI : RVInst16CB<0b100, 0b01, (outs GPRC:$rs1_wb), (ins GPRC:$rs1, simm6:$imm),
+def C_ANDI : RVInst16CB<0b100, 0b01, (outs GPRC:$rd), (ins GPRC:$rs1, simm6:$imm),
                         "c.andi", "$rs1, $imm">,
              Sched<[WriteIALU, ReadIALU]> {
-  let Constraints = "$rs1 = $rs1_wb";
+  let Constraints = "$rs1 = $rd";
   let Inst{12} = imm{5};
   let Inst{11-10} = 0b10;
   let Inst{6-2} = imm{4-0};
@@ -580,11 +580,11 @@ def C_JALR : RVInst16CR<0b1001, 0b10, (outs), (ins GPRNoX0:$rs1),
                         "c.jalr", "$rs1">, Sched<[WriteJalr, ReadJalr]>;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
-def C_ADD : RVInst16CR<0b1001, 0b10, (outs GPRNoX0:$rs1_wb),
+def C_ADD : RVInst16CR<0b1001, 0b10, (outs GPRNoX0:$rd),
                        (ins GPRNoX0:$rs1, GPRNoX0:$rs2),
                        "c.add", "$rs1, $rs2">,
             Sched<[WriteIALU, ReadIALU, ReadIALU]> {
-  let Constraints = "$rs1 = $rs1_wb";
+  let Constraints = "$rs1 = $rd";
 }
 
 let Predicates = [HasStdExtCOrZcd, HasStdExtD] in
@@ -678,11 +678,11 @@ def C_MV_HINT : RVInst16CR<0b1000, 0b10, (outs GPRX0:$rs1), (ins GPRNoX0:$rs2),
   let DecoderMethod = "decodeRVCInstrRdRs2";
 }
 
-def C_ADD_HINT : RVInst16CR<0b1001, 0b10, (outs GPRX0:$rs1_wb),
+def C_ADD_HINT : RVInst16CR<0b1001, 0b10, (outs GPRX0:$rd),
                             (ins GPRX0:$rs1, GPRNoX0:$rs2),
                             "c.add", "$rs1, $rs2">,
                  Sched<[WriteIALU, ReadIALU, ReadIALU]> {
-  let Constraints = "$rs1 = $rs1_wb";
+  let Constraints = "$rs1 = $rd";
   let Inst{11-7} = 0;
   let DecoderMethod = "decodeRVCInstrRdRs1Rs2";
 }

Copy link
Member

@lenary lenary left a comment

Choose a reason for hiding this comment

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

You likely missed C_SRLI64_HINT and C_SRAI64_HINT (noticed from the other PR), but overall this seems to be sensible

@topperc
Copy link
Collaborator Author

topperc commented Oct 14, 2024

You likely missed C_SRLI64_HINT and C_SRAI64_HINT (noticed from the other PR), but overall this seems to be sensible

Yeah. I'll rebase this patch after I push the other one.

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LG

@topperc topperc merged commit 5462725 into llvm:main Oct 15, 2024
8 checks passed
@topperc topperc deleted the pr/rs1_wb branch October 15, 2024 03:49
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
The spec refers to the field as rd'/rs1' so we might as well name the
destination rd.
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
The spec refers to the field as rd'/rs1' so we might as well name the
destination rd.
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
The spec refers to the field as rd'/rs1' so we might as well name the
destination rd.
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