Skip to content

[RISCV] Support Load/Store Global assembler pseudos for Zilsd. #134950

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 1 commit into from
Apr 9, 2025

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Apr 8, 2025

This adds support for 'ld <rd> <symbol>' and 'sd <rd>, <symbol>, <rt>' to match what we do for RV32.

I've changed the interface to emitAuipcInstPair to use MCRegister instead of MCOperand since we need to convert a GPRPair to GPR for TmpReg for the load case.

This adds support for 'ld <rd> <symbol>' and 'sd <rd>, <symbol>, <rt>'
to match what we do for RV32.

I've changed the interface to emitAuipcInstPair to use MCRegister
instead of MCOperand since we need to convert a GPRPair to GPR
for TmpReg for the load case.
@llvmbot llvmbot added backend:RISC-V mc Machine (object) code labels Apr 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 8, 2025

@llvm/pr-subscribers-mc

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

Author: Craig Topper (topperc)

Changes

This adds support for 'ld <rd> <symbol>' and 'sd <rd>, <symbol>, <rt>' to match what we do for RV32.

I've changed the interface to emitAuipcInstPair to use MCRegister instead of MCOperand since we need to convert a GPRPair to GPR for TmpReg for the load case.


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

4 Files Affected:

  • (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+25-12)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrFormats.td (+3-3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoZilsd.td (+3)
  • (added) llvm/test/MC/RISCV/rv32zilsd-pseudos.s (+11)
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index dba78fef0bad8..2d31840d68368 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -135,7 +135,7 @@ class RISCVAsmParser : public MCTargetAsmParser {
 
   // Helper to emit a combination of AUIPC and SecondOpcode. Used to implement
   // helpers such as emitLoadLocalAddress and emitLoadAddress.
-  void emitAuipcInstPair(MCOperand DestReg, MCOperand TmpReg,
+  void emitAuipcInstPair(MCRegister DestReg, MCRegister TmpReg,
                          const MCExpr *Symbol, RISCVMCExpr::Specifier VKHi,
                          unsigned SecondOpcode, SMLoc IDLoc, MCStreamer &Out);
 
@@ -3302,7 +3302,7 @@ void RISCVAsmParser::emitLoadImm(MCRegister DestReg, int64_t Value,
   }
 }
 
-void RISCVAsmParser::emitAuipcInstPair(MCOperand DestReg, MCOperand TmpReg,
+void RISCVAsmParser::emitAuipcInstPair(MCRegister DestReg, MCRegister TmpReg,
                                        const MCExpr *Symbol,
                                        RISCVMCExpr::Specifier VKHi,
                                        unsigned SecondOpcode, SMLoc IDLoc,
@@ -3316,15 +3316,15 @@ void RISCVAsmParser::emitAuipcInstPair(MCOperand DestReg, MCOperand TmpReg,
   Out.emitLabel(TmpLabel);
 
   const RISCVMCExpr *SymbolHi = RISCVMCExpr::create(Symbol, VKHi, Ctx);
-  emitToStreamer(
-      Out, MCInstBuilder(RISCV::AUIPC).addOperand(TmpReg).addExpr(SymbolHi));
+  emitToStreamer(Out,
+                 MCInstBuilder(RISCV::AUIPC).addReg(TmpReg).addExpr(SymbolHi));
 
   const MCExpr *RefToLinkTmpLabel = RISCVMCExpr::create(
       MCSymbolRefExpr::create(TmpLabel, Ctx), RISCVMCExpr::VK_PCREL_LO, Ctx);
 
   emitToStreamer(Out, MCInstBuilder(SecondOpcode)
-                          .addOperand(DestReg)
-                          .addOperand(TmpReg)
+                          .addReg(DestReg)
+                          .addReg(TmpReg)
                           .addExpr(RefToLinkTmpLabel));
 }
 
@@ -3336,7 +3336,7 @@ void RISCVAsmParser::emitLoadLocalAddress(MCInst &Inst, SMLoc IDLoc,
   // expands to
   //   TmpLabel: AUIPC rdest, %pcrel_hi(symbol)
   //             ADDI rdest, rdest, %pcrel_lo(TmpLabel)
-  MCOperand DestReg = Inst.getOperand(0);
+  MCRegister DestReg = Inst.getOperand(0).getReg();
   const MCExpr *Symbol = Inst.getOperand(1).getExpr();
   emitAuipcInstPair(DestReg, DestReg, Symbol, RISCVMCExpr::VK_PCREL_HI,
                     RISCV::ADDI, IDLoc, Out);
@@ -3350,7 +3350,7 @@ void RISCVAsmParser::emitLoadGlobalAddress(MCInst &Inst, SMLoc IDLoc,
   // expands to
   //   TmpLabel: AUIPC rdest, %got_pcrel_hi(symbol)
   //             Lx rdest, %pcrel_lo(TmpLabel)(rdest)
-  MCOperand DestReg = Inst.getOperand(0);
+  MCRegister DestReg = Inst.getOperand(0).getReg();
   const MCExpr *Symbol = Inst.getOperand(1).getExpr();
   unsigned SecondOpcode = isRV64() ? RISCV::LD : RISCV::LW;
   emitAuipcInstPair(DestReg, DestReg, Symbol, RISCVMCExpr::VK_GOT_HI,
@@ -3380,7 +3380,7 @@ void RISCVAsmParser::emitLoadTLSIEAddress(MCInst &Inst, SMLoc IDLoc,
   // expands to
   //   TmpLabel: AUIPC rdest, %tls_ie_pcrel_hi(symbol)
   //             Lx rdest, %pcrel_lo(TmpLabel)(rdest)
-  MCOperand DestReg = Inst.getOperand(0);
+  MCRegister DestReg = Inst.getOperand(0).getReg();
   const MCExpr *Symbol = Inst.getOperand(1).getExpr();
   unsigned SecondOpcode = isRV64() ? RISCV::LD : RISCV::LW;
   emitAuipcInstPair(DestReg, DestReg, Symbol, RISCVMCExpr::VK_TLS_GOT_HI,
@@ -3395,7 +3395,7 @@ void RISCVAsmParser::emitLoadTLSGDAddress(MCInst &Inst, SMLoc IDLoc,
   // expands to
   //   TmpLabel: AUIPC rdest, %tls_gd_pcrel_hi(symbol)
   //             ADDI rdest, rdest, %pcrel_lo(TmpLabel)
-  MCOperand DestReg = Inst.getOperand(0);
+  MCRegister DestReg = Inst.getOperand(0).getReg();
   const MCExpr *Symbol = Inst.getOperand(1).getExpr();
   emitAuipcInstPair(DestReg, DestReg, Symbol, RISCVMCExpr::VK_TLS_GD_HI,
                     RISCV::ADDI, IDLoc, Out);
@@ -3412,9 +3412,16 @@ void RISCVAsmParser::emitLoadStoreSymbol(MCInst &Inst, unsigned Opcode,
   //   TmpLabel: AUIPC tmp, %pcrel_hi(symbol)
   //             [S|L]X    rd, %pcrel_lo(TmpLabel)(tmp)
   unsigned DestRegOpIdx = HasTmpReg ? 1 : 0;
-  MCOperand DestReg = Inst.getOperand(DestRegOpIdx);
+  MCRegister DestReg = Inst.getOperand(DestRegOpIdx).getReg();
   unsigned SymbolOpIdx = HasTmpReg ? 2 : 1;
-  MCOperand TmpReg = Inst.getOperand(0);
+  MCRegister TmpReg = Inst.getOperand(0).getReg();
+
+  // If TmpReg is a GPR pair, get the even register.
+  if (RISCVMCRegisterClasses[RISCV::GPRPairRegClassID].contains(TmpReg)) {
+    const MCRegisterInfo *RI = getContext().getRegisterInfo();
+    TmpReg = RI->getSubReg(TmpReg, RISCV::sub_gpr_even);
+  }
+
   const MCExpr *Symbol = Inst.getOperand(SymbolOpIdx).getExpr();
   emitAuipcInstPair(DestReg, TmpReg, Symbol, RISCVMCExpr::VK_PCREL_HI, Opcode,
                     IDLoc, Out);
@@ -3766,6 +3773,9 @@ bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
   case RISCV::PseudoLD:
     emitLoadStoreSymbol(Inst, RISCV::LD, IDLoc, Out, /*HasTmpReg=*/false);
     return false;
+  case RISCV::PseudoLD_RV32:
+    emitLoadStoreSymbol(Inst, RISCV::LD_RV32, IDLoc, Out, /*HasTmpReg=*/false);
+    return false;
   case RISCV::PseudoFLH:
     emitLoadStoreSymbol(Inst, RISCV::FLH, IDLoc, Out, /*HasTmpReg=*/true);
     return false;
@@ -3787,6 +3797,9 @@ bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
   case RISCV::PseudoSD:
     emitLoadStoreSymbol(Inst, RISCV::SD, IDLoc, Out, /*HasTmpReg=*/true);
     return false;
+  case RISCV::PseudoSD_RV32:
+    emitLoadStoreSymbol(Inst, RISCV::SD_RV32, IDLoc, Out, /*HasTmpReg=*/true);
+    return false;
   case RISCV::PseudoFSH:
     emitLoadStoreSymbol(Inst, RISCV::FSH, IDLoc, Out, /*HasTmpReg=*/true);
     return false;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrFormats.td b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
index 0bb0ba57ff50d..8d56637d8b281 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrFormats.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
@@ -301,8 +301,8 @@ class PseudoQuietFCMP<DAGOperand Ty>
 }
 
 // Pseudo load instructions.
-class PseudoLoad<string opcodestr>
-    : Pseudo<(outs GPR:$rd), (ins bare_symbol:$addr), [], opcodestr, "$rd, $addr"> {
+class PseudoLoad<string opcodestr, DAGOperand rdty = GPR>
+    : Pseudo<(outs rdty:$rd), (ins bare_symbol:$addr), [], opcodestr, "$rd, $addr"> {
   let hasSideEffects = 0;
   let mayLoad = 1;
   let mayStore = 0;
@@ -320,7 +320,7 @@ class PseudoFloatLoad<string opcodestr, RegisterClass rdty>
 }
 
 // Pseudo store instructions.
-class PseudoStore<string opcodestr, RegisterClass rsty = GPR>
+class PseudoStore<string opcodestr, DAGOperand rsty = GPR>
     : Pseudo<(outs GPR:$tmp), (ins rsty:$rs, bare_symbol:$addr), [], opcodestr, "$rs, $addr, $tmp"> {
   let hasSideEffects = 0;
   let mayLoad = 0;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZilsd.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZilsd.td
index d0d37c1c90e12..98889964d1299 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoZilsd.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZilsd.td
@@ -42,6 +42,9 @@ def SD_RV32 : PairStore_rri<"sd", GPRPairRV32>, Sched<[WriteSTD, ReadStoreData,
 //===----------------------------------------------------------------------===//
 
 let Predicates = [HasStdExtZilsd, IsRV32] in {
+def PseudoLD_RV32 : PseudoLoad<"ld", GPRPairRV32>;
+def PseudoSD_RV32 : PseudoStore<"sd", GPRPairRV32>;
+
 def : InstAlias<"ld $rd, (${rs1})", (LD_RV32 GPRPairRV32:$rd, GPR:$rs1, 0), 0>;
 def : InstAlias<"sd $rs2, (${rs1})", (SD_RV32 GPRPairRV32:$rs2, GPR:$rs1, 0), 0>;
 }
diff --git a/llvm/test/MC/RISCV/rv32zilsd-pseudos.s b/llvm/test/MC/RISCV/rv32zilsd-pseudos.s
new file mode 100644
index 0000000000000..fda56aadea25a
--- /dev/null
+++ b/llvm/test/MC/RISCV/rv32zilsd-pseudos.s
@@ -0,0 +1,11 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zilsd | FileCheck %s
+
+# CHECK: .Lpcrel_hi0:
+# CHECK: auipc a4, %pcrel_hi(a_symbol)
+# CHECK: ld  a4, %pcrel_lo(.Lpcrel_hi0)(a4)
+ld a4, a_symbol
+
+# CHECK: .Lpcrel_hi1:
+# CHECK: auipc a3, %pcrel_hi(a_symbol)
+# CHECK: sd  a6, %pcrel_lo(.Lpcrel_hi1)(a3)
+sd a6, a_symbol, a3

@topperc
Copy link
Collaborator Author

topperc commented Apr 8, 2025

@lenary my incorrect reading of "load/store pair" in your comment from #134931 combined with the word pseudo magically made me realize we had forgotten to do this.

@topperc
Copy link
Collaborator Author

topperc commented Apr 8, 2025

cc: @dong-miao

@topperc topperc requested a review from monkchiang April 8, 2025 23:57
Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

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

LGTM, and just FYI, binutils's patch also support this, it's not landed yet, but will landing soon I think.

@lenary
Copy link
Member

lenary commented Apr 9, 2025

I'll do a PR to add these to the asm manual with the others.

@topperc
Copy link
Collaborator Author

topperc commented Apr 9, 2025

I'll do a PR to add these to the asm manual with the others.

Thanks.

@lenary
Copy link
Member

lenary commented Apr 9, 2025

@topperc topperc merged commit 5c27511 into llvm:main Apr 9, 2025
14 checks passed
@topperc topperc deleted the pr/zilsd-pseudo branch April 9, 2025 04:33
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…134950)

This adds support for 'ld \<rd\> \<symbol\>' and 'sd \<rd\>, \<symbol\>,
\<rt\>' to match what we do for RV32.

I've changed the interface to emitAuipcInstPair to use MCRegister
instead of MCOperand since we need to convert a GPRPair to GPR for
TmpReg for the load case.
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.

4 participants