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] Convert C_ADDI_NOP to C_NOP in the assembler. #112314

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Oct 15, 2024

Make it a pseudoinstruction so we can convert it to C_NOP. This makes the printing from the assembler consistent with what we get from llvm-objdump.

I tried to do this with an InstAlias, but I don't think it can drop operands.

Make it a pseudoinstruction so we can convert it to C_NOP. This
makes the printing from the assembler consistent with what we
get from llvm-objdump.

I tried to this with an InstAlias, but I don't think it can drop
operands.
@llvmbot llvmbot added backend:RISC-V mc Machine (object) code labels Oct 15, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2024

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

Author: Craig Topper (topperc)

Changes

Make it a pseudoinstruction so we can convert it to C_NOP. This makes the printing from the assembler consistent with what we get from llvm-objdump.

I tried to this with an InstAlias, but I don't think it can drop operands.


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

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+5-9)
  • (modified) llvm/test/MC/RISCV/rv32c-valid.s (+1-2)
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index d77ad02ec47bf1..0bc35846627c0f 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -3693,6 +3693,9 @@ bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
   switch (Inst.getOpcode()) {
   default:
     break;
+  case RISCV::PseudoC_ADDI_NOP:
+    emitToStreamer(Out, MCInstBuilder(RISCV::C_NOP));
+    return false;
   case RISCV::PseudoLLAImm:
   case RISCV::PseudoLAImm:
   case RISCV::PseudoLI: {
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index e8c4860fd3e55e..8a76dba23d42a7 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -418,15 +418,11 @@ def C_ADDI : RVInst16CI<0b000, 0b01, (outs GPRNoX0:$rd_wb),
   let Inst{6-2} = imm{4-0};
 }
 
-let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
-def C_ADDI_NOP : RVInst16CI<0b000, 0b01, (outs GPRX0:$rd_wb),
-                            (ins GPRX0:$rd, immzero:$imm),
-                            "c.addi", "$rd, $imm">,
-                 Sched<[WriteIALU, ReadIALU]> {
-  let Constraints = "$rd = $rd_wb";
-  let Inst{6-2} = 0;
-  let isAsmParserOnly = 1;
-}
+// Alternate syntax for c.nop. Converted to C_NOP by the assembler.
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0, isCodeGenOnly = 0,
+    isAsmParserOnly = 1 in
+def PseudoC_ADDI_NOP : Pseudo<(outs GPRX0:$rd), (ins GPRX0:$rs1, immzero:$imm),
+                              [], "c.addi", "$rd, $imm">;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0, isCall = 1,
     DecoderNamespace = "RISCV32Only_", Defs = [X1],
diff --git a/llvm/test/MC/RISCV/rv32c-valid.s b/llvm/test/MC/RISCV/rv32c-valid.s
index bcdf27a2ba783b..9b0ca80a7adc20 100644
--- a/llvm/test/MC/RISCV/rv32c-valid.s
+++ b/llvm/test/MC/RISCV/rv32c-valid.s
@@ -147,8 +147,7 @@ c.sub a4, a5
 # CHECK-ASM: encoding: [0x01,0x00]
 # CHECK-NO-EXT:  error: instruction requires the following: 'C' (Compressed Instructions) or 'Zca' (part of the C extension, excluding compressed floating point loads/stores){{$}}
 c.nop
-# CHECK-ASM: c.addi zero, 0
-# CHECK-OBJ: c.nop
+# CHECK-ASM-AND-OBJ: c.nop
 # CHECK-ASM: encoding: [0x01,0x00]
 # CHECK-NO-EXT:  error: instruction requires the following: 'C' (Compressed Instructions) or 'Zca' (part of the C extension, excluding compressed floating point loads/stores){{$}}
 c.addi x0, 0

@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2024

@llvm/pr-subscribers-mc

Author: Craig Topper (topperc)

Changes

Make it a pseudoinstruction so we can convert it to C_NOP. This makes the printing from the assembler consistent with what we get from llvm-objdump.

I tried to this with an InstAlias, but I don't think it can drop operands.


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

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoC.td (+5-9)
  • (modified) llvm/test/MC/RISCV/rv32c-valid.s (+1-2)
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index d77ad02ec47bf1..0bc35846627c0f 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -3693,6 +3693,9 @@ bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
   switch (Inst.getOpcode()) {
   default:
     break;
+  case RISCV::PseudoC_ADDI_NOP:
+    emitToStreamer(Out, MCInstBuilder(RISCV::C_NOP));
+    return false;
   case RISCV::PseudoLLAImm:
   case RISCV::PseudoLAImm:
   case RISCV::PseudoLI: {
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
index e8c4860fd3e55e..8a76dba23d42a7 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoC.td
@@ -418,15 +418,11 @@ def C_ADDI : RVInst16CI<0b000, 0b01, (outs GPRNoX0:$rd_wb),
   let Inst{6-2} = imm{4-0};
 }
 
-let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
-def C_ADDI_NOP : RVInst16CI<0b000, 0b01, (outs GPRX0:$rd_wb),
-                            (ins GPRX0:$rd, immzero:$imm),
-                            "c.addi", "$rd, $imm">,
-                 Sched<[WriteIALU, ReadIALU]> {
-  let Constraints = "$rd = $rd_wb";
-  let Inst{6-2} = 0;
-  let isAsmParserOnly = 1;
-}
+// Alternate syntax for c.nop. Converted to C_NOP by the assembler.
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0, isCodeGenOnly = 0,
+    isAsmParserOnly = 1 in
+def PseudoC_ADDI_NOP : Pseudo<(outs GPRX0:$rd), (ins GPRX0:$rs1, immzero:$imm),
+                              [], "c.addi", "$rd, $imm">;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0, isCall = 1,
     DecoderNamespace = "RISCV32Only_", Defs = [X1],
diff --git a/llvm/test/MC/RISCV/rv32c-valid.s b/llvm/test/MC/RISCV/rv32c-valid.s
index bcdf27a2ba783b..9b0ca80a7adc20 100644
--- a/llvm/test/MC/RISCV/rv32c-valid.s
+++ b/llvm/test/MC/RISCV/rv32c-valid.s
@@ -147,8 +147,7 @@ c.sub a4, a5
 # CHECK-ASM: encoding: [0x01,0x00]
 # CHECK-NO-EXT:  error: instruction requires the following: 'C' (Compressed Instructions) or 'Zca' (part of the C extension, excluding compressed floating point loads/stores){{$}}
 c.nop
-# CHECK-ASM: c.addi zero, 0
-# CHECK-OBJ: c.nop
+# CHECK-ASM-AND-OBJ: c.nop
 # CHECK-ASM: encoding: [0x01,0x00]
 # CHECK-NO-EXT:  error: instruction requires the following: 'C' (Compressed Instructions) or 'Zca' (part of the C extension, excluding compressed floating point loads/stores){{$}}
 c.addi x0, 0

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.

@topperc topperc merged commit 0850e72 into llvm:main Oct 16, 2024
11 checks passed
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
Make it a pseudoinstruction so we can convert it to C_NOP. This makes
the printing from the assembler consistent with what we get from
llvm-objdump.

I tried to do this with an InstAlias, but I don't think it can drop
operands.
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
Make it a pseudoinstruction so we can convert it to C_NOP. This makes
the printing from the assembler consistent with what we get from
llvm-objdump.

I tried to do this with an InstAlias, but I don't think it can drop
operands.
AnastasiyaChernikova added a commit to AnastasiyaChernikova/llvm-project that referenced this pull request Oct 28, 2024
AnastasiyaChernikova added a commit to AnastasiyaChernikova/llvm-project that referenced this pull request Oct 30, 2024
AnastasiyaChernikova added a commit to AnastasiyaChernikova/llvm-project that referenced this pull request Oct 31, 2024
AnastasiyaChernikova added a commit to AnastasiyaChernikova/llvm-project that referenced this pull request Nov 26, 2024
@topperc topperc deleted the pr/addi-nop branch December 10, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants