[RISCV] Add assembler support for LA pseudo-instruction

[rogfer01]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1483

My understanding is that .option pic and .option nopic change this behaviour.

A quick look at MCObjectFileInfo class files shows that the attribute is only set during the construction of the object and can't be changed.

In my downstream I implemented .option pic and .option nopic which set a field of RISCVAsmParser that I called IsPicEnabled and is only used for la. That field is initialised in the constructor using getContext().getObjectFileInfo()->isPositionIndependent(). Perhaps adding a setter to MCObjectFileInfo might be a better approach.

That said, either we implement .option pic / .option nopic first or we add a FIXME here and implement it later.

Hope this makes sense.

[jrtc27]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1483

Indeed, you're absolutely right, and I have what sounds like nearly-identical code in my own local repository. Given that, as far as I know, .option pic/nopic only affects the expansion of la, I decided it made sense to add la first rather than a seemingly-pointless option. I'm not sure whether a FIXME is really necessary; anyone adding .option pic/nopic support must look for PIC-related checks, and we have all the code, it's just a case of changing the condition (unlike some cases where FIXMEs do exist in code gen because the expansion just doesn't exist).

[rogfer01]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1483

Agreed. The FIXME may not help much here. Thanks @jrtc27 !

[jrtc27]

Rebased

[lewis-revill]

lib/Target/RISCV/RISCVInstrInfo.td

486

Could you remove this for completeness? Not sure why it's all the way up here.

[lewis-revill]

lib/Target/RISCV/RISCVInstrInfo.td

823

Nitpick: this could be indented by one less space.

[asb]

Thanks, this basically looks good to me. Could you please add some tests for la with invalid operands.

Not hugely important, but I do think that adding a FIXME/TODO as Lewis suggests would be worthwhile. It signals to anyone reading the code that they're encountering a known limitation rather than the author failing to consider the requirements of .option pic/nopic.

test/MC/RISCV/rvi-pseudos.s

37

Should strip the unwanted whitespace in this file

[asb]

Hi James. Might you have time to address the outstanding (minor) review comments?

Additionally, can you confirm that you are aware of the new license put in place last Friday and intend to contribute under it. See http://lists.llvm.org/pipermail/llvm-dev/2019-January/129344.html https://llvm.org/docs/DeveloperPolicy.html#new-llvm-project-license-framework

Thanks!

[jrtc27]

Rebased, fixed whitespace, removed TODO, added FIXME, added invalid tests.

[asb]

Looks good to me - thanks. I like the refactoring to introduce emitAuipcInstPair. Nit: a comment next to its declaration (along the lines of the comments for neighbouring helper declarations) would be useful.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstBuilder.h"
+#include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/MathExtras.h"
class RISCVAsmParser : public MCTargetAsmParser {
   // Helper to actually emit an instruction to the MCStreamer. Also, when
   // possible, compression of the instruction is performed.
   void emitToStreamer(MCStreamer &S, const MCInst &Inst);
 
   // Helper to emit a combination of LUI, ADDI(W), and SLLI instructions that
   // synthesize the desired immedate value into the destination register.
   void emitLoadImm(unsigned DestReg, int64_t Value, MCStreamer &Out);
 
+  void emitAuipcInstPair(MCOperand DestReg, MCOperand TmpReg,
+                         const MCExpr *Symbol, RISCVMCExpr::VariantKind VKHi,
+                         unsigned SecondOpcode, SMLoc IDLoc, MCStreamer &Out);
+
   // Helper to emit pseudo instruction "lla" used in PC-rel addressing.
   void emitLoadLocalAddress(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out);
 
+  // Helper to emit pseudo instruction "la" used in GOT/PC-rel addressing.
+  void emitLoadAddress(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out);
+
   /// Helper for processing MC instructions that have been successfully matched
   /// by MatchAndEmitInstruction. Modifications to the emitted instructions,
   /// like the expansion of pseudo instructions (e.g., "li"), can be performed
   /// in this method.
   bool processInstruction(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out);
 
 // Auto-generated instruction matching functions
 #define GET_ASSEMBLER_HEADER
if (Inst.Opc == RISCV::LUI) {
                    Inst.Imm));
     }
 
     // Only the first instruction has X0 as its source.
     SrcReg = DestReg;
   }
 }
 
-void RISCVAsmParser::emitLoadLocalAddress(MCInst &Inst, SMLoc IDLoc,
-                                          MCStreamer &Out) {
-  // The local load address pseudo-instruction "lla" is used in PC-relative
-  // addressing of symbols:
-  //   lla rdest, symbol
-  // expands to
-  //   TmpLabel: AUIPC rdest, %pcrel_hi(symbol)
-  //             ADDI rdest, %pcrel_lo(TmpLabel)
+void RISCVAsmParser::emitAuipcInstPair(MCOperand DestReg, MCOperand TmpReg,
+                                       const MCExpr *Symbol,
+                                       RISCVMCExpr::VariantKind VKHi,
+                                       unsigned SecondOpcode, SMLoc IDLoc,
+  // A pair of instructions for PC-relative addressing; expands to
+  //   TmpLabel: AUIPC TmpReg, VKHi(symbol)
+  //             OP DestReg, TmpReg, %pcrel_lo(TmpLabel)
   MCContext &Ctx = getContext();
 
   MCSymbol *TmpLabel = Ctx.createTempSymbol(
       "pcrel_hi", /* AlwaysAddSuffix */ true, /* CanBeUnnamed */ false);
   Out.EmitLabel(TmpLabel);
 
-  MCOperand DestReg = Inst.getOperand(0);
-  const RISCVMCExpr *Symbol = RISCVMCExpr::create(
-      Inst.getOperand(1).getExpr(), RISCVMCExpr::VK_RISCV_PCREL_HI, Ctx);
-
+  const RISCVMCExpr *SymbolHi = RISCVMCExpr::create(Symbol, VKHi, Ctx);
   emitToStreamer(
-      Out, MCInstBuilder(RISCV::AUIPC).addOperand(DestReg).addExpr(Symbol));
+      Out, MCInstBuilder(RISCV::AUIPC).addOperand(TmpReg).addExpr(SymbolHi));
 
   const MCExpr *RefToLinkTmpLabel =
       RISCVMCExpr::create(MCSymbolRefExpr::create(TmpLabel, Ctx),
                           RISCVMCExpr::VK_RISCV_PCREL_LO, Ctx);
 
-  emitToStreamer(Out, MCInstBuilder(RISCV::ADDI)
-                          .addOperand(DestReg)
+  emitToStreamer(Out, MCInstBuilder(SecondOpcode)
                           .addOperand(DestReg)
+                          .addOperand(TmpReg)
                           .addExpr(RefToLinkTmpLabel));
 }
 
+void RISCVAsmParser::emitLoadLocalAddress(MCInst &Inst, SMLoc IDLoc,
+                                          MCStreamer &Out) {
+  // The load local address pseudo-instruction "lla" is used in PC-relative
+  // addressing of local symbols:
+  //   lla rdest, symbol
+  // expands to
+  //   TmpLabel: AUIPC rdest, %pcrel_hi(symbol)
+  //             ADDI rdest, rdest, %pcrel_lo(TmpLabel)
+  MCOperand DestReg = Inst.getOperand(0);
+  const MCExpr *Symbol = Inst.getOperand(1).getExpr();
+  emitAuipcInstPair(DestReg, DestReg, Symbol, RISCVMCExpr::VK_RISCV_PCREL_HI,
+                    RISCV::ADDI, IDLoc, Out);
+}
+
+void RISCVAsmParser::emitLoadAddress(MCInst &Inst, SMLoc IDLoc,
+                                     MCStreamer &Out) {
+  // The load address pseudo-instruction "la" is used in PC-relative and
+  // GOT-indirect addressing of global symbols:
+  //   la rdest, symbol
+  // expands to either (for non-PIC)
+  //   TmpLabel: AUIPC rdest, %pcrel_hi(symbol)
+  //             ADDI rdest, rdest, %pcrel_lo(TmpLabel)
+  // or (for PIC)
+  //   TmpLabel: AUIPC rdest, %got_pcrel_hi(symbol)
+  //             Lx rdest, %pcrel_lo(TmpLabel)(rdest)
+  MCOperand DestReg = Inst.getOperand(0);
+  const MCExpr *Symbol = Inst.getOperand(1).getExpr();
+  unsigned SecondOpcode;
+  RISCVMCExpr::VariantKind VKHi;
+  if (getContext().getObjectFileInfo()->isPositionIndependent()) {
+    SecondOpcode = isRV64() ? RISCV::LD : RISCV::LW;
+    VKHi = RISCVMCExpr::VK_RISCV_GOT_HI;
+  } else {
+    SecondOpcode = RISCV::ADDI;
+    VKHi = RISCVMCExpr::VK_RISCV_PCREL_HI;
+  }
+  emitAuipcInstPair(DestReg, DestReg, Symbol, VKHi, SecondOpcode, IDLoc, Out);
+}
+
 bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
                                         MCStreamer &Out) {
   Inst.setLoc(IDLoc);
 
-  if (Inst.getOpcode() == RISCV::PseudoLI) {
+  switch (Inst.getOpcode()) {
+  default:
+    break;
+  case RISCV::PseudoLI: {
     auto Reg = Inst.getOperand(0).getReg();
     int64_t Imm = Inst.getOperand(1).getImm();
     // On RV32 the immediate here can either be a signed or an unsigned
     // 32-bit number. Sign extension has to be performed to ensure that Imm
     // represents the expected signed 64-bit number.
     if (!isRV64())
       Imm = SignExtend64<32>(Imm);
     emitLoadImm(Reg, Imm, Out);
     return false;
-  } else if (Inst.getOpcode() == RISCV::PseudoLLA) {
+  }
+  case RISCV::PseudoLLA:
     emitLoadLocalAddress(Inst, IDLoc, Out);
     return false;
+  case RISCV::PseudoLA:
+    emitLoadAddress(Inst, IDLoc, Out);
+    return false;
   }
 
   emitToStreamer(Out, Inst);
   return false;
 }
 
 extern "C" void LLVMInitializeRISCVAsmParser() {
   RegisterMCAsmParser<RISCVAsmParser> X(getTheRISCV32Target());
   RegisterMCAsmParser<RISCVAsmParser> Y(getTheRISCV64Target());
 }

lib/Target/RISCV/RISCVInstrInfo.td

def SFENCE_VMA : RVInstR<0b0001001, 0b000, OPC_SYSTEM, (outs),
                          "sfence.vma", "$rs1, $rs2"> {
   let rd = 0;
 }
 
 //===----------------------------------------------------------------------===//
 // Assembler Pseudo Instructions (User-Level ISA, Version 2.2, Chapter 20)
 //===----------------------------------------------------------------------===//
 
 // TODO la
 // TODO lb lh lw
 // TODO RV64I: ld
 // TODO sb sh sw
 // TODO RV64I: sd
 
 def : InstAlias<"nop",           (ADDI      X0,      X0,       0)>;
 
 // Note that the size is 32 because up to 8 32-bit instructions are needed to

 def : Pat<(Tail (iPTR texternalsym:$dst)),
           (PseudoTAIL texternalsym:$dst)>;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0, isCodeGenOnly = 0,
     isAsmParserOnly = 1 in
 def PseudoLLA : Pseudo<(outs GPR:$dst), (ins bare_symbol:$src), [],
                        "lla", "$dst, $src">;
 
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0, isCodeGenOnly = 0,
+    isAsmParserOnly = 1 in
+def PseudoLA : Pseudo<(outs GPR:$dst), (ins bare_symbol:$src), [],
+                       "la", "$dst, $src">;
+
 /// Loads
 
 multiclass LdPat<PatFrag LoadOp, RVInst Inst> {
   def : Pat<(LoadOp GPR:$rs1), (Inst GPR:$rs1, 0)>;
   def : Pat<(LoadOp AddrFI:$rs1), (Inst AddrFI:$rs1, 0)>;
   def : Pat<(LoadOp (add GPR:$rs1, simm12:$imm12)),
             (Inst GPR:$rs1, simm12:$imm12)>;
   def : Pat<(LoadOp (add AddrFI:$rs1, simm12:$imm12)),

test/MC/RISCV/rvi-pseudos.s

-# RUN: llvm-mc %s -triple=riscv32 | FileCheck %s
-# RUN: llvm-mc %s -triple=riscv64 | FileCheck %s
+# RUN: llvm-mc %s -triple=riscv32 | FileCheck %s --check-prefixes=CHECK,CHECK-NOPIC
+# RUN: llvm-mc %s -triple=riscv64 | FileCheck %s --check-prefixes=CHECK,CHECK-NOPIC
+# RUN: llvm-mc %s -triple=riscv32 -position-independent \
+# RUN:   | FileCheck %s --check-prefixes=CHECK,CHECK-PIC,CHECK-PIC-RV32
+# RUN: llvm-mc %s -triple=riscv64 -position-independent \
+# RUN:   | FileCheck %s --check-prefixes=CHECK,CHECK-PIC,CHECK-PIC-RV64
 
 # CHECK: .Lpcrel_hi0:
 # CHECK: auipc a0, %pcrel_hi(a_symbol)
 # CHECK: addi  a0, a0, %pcrel_lo(.Lpcrel_hi0)
 lla a0, a_symbol
 
 # CHECK: .Lpcrel_hi1:
 # CHECK: auipc a1, %pcrel_hi(another_symbol)

 # CHECK: auipc a3, %pcrel_hi(ra)
 # CHECK: addi  a3, a3, %pcrel_lo(.Lpcrel_hi3)
 lla a3, ra
 
 # CHECK: .Lpcrel_hi4:
 # CHECK: auipc a4, %pcrel_hi(f1)
 # CHECK: addi  a4, a4, %pcrel_lo(.Lpcrel_hi4)
 lla a4, f1
+
+# CHECK: .Lpcrel_hi5:
+# CHECK-NOPIC: auipc a0, %pcrel_hi(a_symbol)
+# CHECK-NOPIC: addi  a0, a0, %pcrel_lo(.Lpcrel_hi5)
+# CHECK-PIC:       auipc a0, %got_pcrel_hi(a_symbol)
+# CHECK-PIC-RV32: lw    a0, %pcrel_lo(.Lpcrel_hi5)(a0)
+# CHECK-PIC-RV64: ld    a0, %pcrel_lo(.Lpcrel_hi5)(a0)
+la a0, a_symbol
+
+# CHECK: .Lpcrel_hi6:
+# CHECK-NOPIC: auipc a1, %pcrel_hi(another_symbol)
+# CHECK-NOPIC: addi  a1, a1, %pcrel_lo(.Lpcrel_hi6)
+# CHECK-PIC:       auipc a1, %got_pcrel_hi(another_symbol)
+# CHECK-PIC-RV32: lw    a1, %pcrel_lo(.Lpcrel_hi6)(a1)
+# CHECK-PIC-RV64: ld    a1, %pcrel_lo(.Lpcrel_hi6)(a1)
+la a1, another_symbol
+
+# Check that we can load the address of symbols that are spelled like a register
+# CHECK: .Lpcrel_hi7:
+# CHECK-NOPIC: auipc a2, %pcrel_hi(zero)
+# CHECK-NOPIC: addi  a2, a2, %pcrel_lo(.Lpcrel_hi7)
+# CHECK-PIC:       auipc a2, %got_pcrel_hi(zero)
+# CHECK-PIC-RV32: lw    a2, %pcrel_lo(.Lpcrel_hi7)(a2)
+# CHECK-PIC-RV64: ld    a2, %pcrel_lo(.Lpcrel_hi7)(a2)
+la a2, zero
+
+# CHECK: .Lpcrel_hi8:
+# CHECK-NOPIC: auipc a3, %pcrel_hi(ra)
+# CHECK-NOPIC: addi  a3, a3, %pcrel_lo(.Lpcrel_hi8)
+# CHECK-PIC:       auipc a3, %got_pcrel_hi(ra)
+# CHECK-PIC-RV32: lw    a3, %pcrel_lo(.Lpcrel_hi8)(a3)
+# CHECK-PIC-RV64: ld    a3, %pcrel_lo(.Lpcrel_hi8)(a3)
+la a3, ra
+
+# CHECK: .Lpcrel_hi9:
+# CHECK-NOPIC: auipc a4, %pcrel_hi(f1)
+# CHECK-NOPIC: addi  a4, a4, %pcrel_lo(.Lpcrel_hi9)
+# CHECK-PIC:       auipc a4, %got_pcrel_hi(f1)
+# CHECK-PIC-RV32: lw    a4, %pcrel_lo(.Lpcrel_hi9)(a4)
+# CHECK-PIC-RV64: ld    a4, %pcrel_lo(.Lpcrel_hi9)(a4)
+la a4, f1