[RISCV] Add "lla" pseudo-instruction to assembler

[rogfer01]

Summary

This pseudo-instruction is similar to la but uses PC-relative addressing unconditionally. This is, la is only different to lla when using -fPIC. This pseudo-instruction seems often forgotten in several specs but it is definitely mentioned in binutils opcodes/riscv-opc.c. The semantics are defined both in page 37 of the "RISC-V Reader" book but also in function macro found in gas/config/tc-riscv.c.

This is a very first step towards adding PIC support for Linux in the RISC-V backend.

The lla pseudo-instruction expands to a sequence of auipc + addi with a couple of pc-rel relocations where the second points to the first one. This is described in https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#pc-relative-symbol-addresses

For now, this patch only introduces support of that pseudo instruction at the assembler parser.

As part of the remaining PIC changes and to avoid code duplication between the codegen flow and the assembler flow, I plan to lower PC-rel addressing (the one that does not use the GOT, e.g. loading a file-scope static variable) into a PseudoLLA. But this means the expansion of the pseudo-instruction still has to happen elsewhere for the codegen flow. Again to avoid code duplication, my idea was to introduce a RISCVELFStreamer which knows how to expand the pseudo-MC-instruction when generating an object (I'm a bit worried though that I might be tying this to ELF). I'd do a similar thing with la (but that will require some more stuff not in upstream yet).

I considered using MCCodeEmitter as we do now for PseudoCALL and PseudoTAIL but I don't think this can work as I need to emit a label, something that the MCCodeEmitter doesn't seem (to the best of my knowledge) able to do.

Hope my plan above makes sense.

[asb]

Thanks for the patch, just a few minor comments in-line.

Could you add a test that shows non-bare symbols are rejected? e.g. lla a0, %lo(sym)?

This will fail to parse lla where the symbol coincides with a register name. Take a look at 'ForceImmediate' as used when parsing call and tail - you'll need to do something similar here. Due to the lack of clear separation between the register and symbol name namespaces for these pseudo-instructions, I think this kind of hack in the parser is the only way to go.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1192 ↗

(On Diff #156733)

Could you make the comment more descriptive? e.g. explaining the load local address pseudo-instruction.

1214–1216 ↗

(On Diff #156733)

clang-format prefers slightly different indentation here

lib/Target/RISCV/RISCVInstrInfo.td

715–717 ↗

(On Diff #156733)

It would be slightly more consistent with other instruction definitions to write this as:

def PseudoLLA : Pseudo<(outs GPR:$dst), (ins bare_symbol:$src), [],
                       "lla", "$dst, $src">;

[rogfer01]

Thanks a lot for the review @asb !

ChangeLog:

• Refactor a bit ForceImmediate flag to include lla in the second operand
• Extend test to include symbols named like registers
• New negative test for non-bare symbols
• Addressed code consistency issues. Update comment.

[asb]

Thanks for the update. The helper function for force immediate is a good idea - just a couple of minor comments there.

It would be nice if there was a sensible way to merge the tests in to rvi-aliases-valid.s and a new rvi-aliases-invalid.s, but I see that CHECK-EXPAND (as used by li) isn't appropriate for lla, because of course we won't see %pcrel_hi/%pcrel_lo in the objdump output.

With the suggested tweaks to the helper function I think this is good to go. Thanks!

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

969 ↗

(On Diff #157647)

I think we'd benefit from a comment here describing the function. How about:

Return true if the operand at the OperandIdx for opcode Name should be 'forced' to be parsed as an immediate. This is required for pseudoinstructions such as tail or call, which allow bare symbols to be used that could clash with register names.

970 ↗

(On Diff #157647)

Function names should start with a lower-case letter. ParseInstruction etc don't do this as they override an existing method, which doesn't conform with this naming convention. https://llvm.org/docs/CodingStandards.html#id42. Maybe shouldForceImediateOperand would be better given the bool return type?

[rogfer01]

Thanks @asb for the review!

ChangeLog:

• Rename ForceImmediateOperand → shouldForceImediateOperand.
• Add comment to shouldForceImediateOperand
• Fix mayLoad it can be mayLoad = 0 because this is like li, no memory is actually read.
• Update comments to avoid suggesting that lla loads memory.

[asb]

Thanks, this looks good to me.

[rogfer01]

Thanks for the review @asb . I will commit soon.

llvm/trunk/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

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);
 
+  // Helper to emit pseudo instruction "lla" used in PC-rel addressing.
+  void emitLoadLocalAddress(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 (parseImmediate(Operands) == MatchOperand_Success) {
     return false;
   }
 
   // Finally we have exhausted all options and must declare defeat.
   Error(getLoc(), "unknown operand");
   return true;
 }
 
+/// Return true if the operand at the OperandIdx for opcode Name should be
+/// 'forced' to be parsed as an immediate. This is required for
+/// pseudoinstructions such as tail or call, which allow bare symbols to be used
+/// that could clash with register names.
+static bool shouldForceImediateOperand(StringRef Name, unsigned OperandIdx) {
+  // FIXME: This may not scale so perhaps we want to use a data-driven approach
+  // instead.
+  switch (OperandIdx) {
+  case 0:
+    // call imm
+    // tail imm
+    return Name == "tail" || Name == "call";
+  case 1:
+    // lla rdest, imm
+    return Name == "lla";
+  default:
+    return false;
+  }
+}
+
 bool RISCVAsmParser::ParseInstruction(ParseInstructionInfo &Info,
                                       StringRef Name, SMLoc NameLoc,
                                       OperandVector &Operands) {
   // First operand is token for instruction
   Operands.push_back(RISCVOperand::createToken(Name, NameLoc, isRV64()));
 
   // If there are no more operands, then finish
   if (getLexer().is(AsmToken::EndOfStatement))
     return false;
 
   // Parse first operand
-  bool ForceImmediate = (Name == "call" || Name == "tail");
-  if (parseOperand(Operands, ForceImmediate))
+  if (parseOperand(Operands, shouldForceImediateOperand(Name, 0)))
     return true;
 
   // Parse until end of statement, consuming commas between operands
+  unsigned OperandIdx = 1;
   while (getLexer().is(AsmToken::Comma)) {
     // Consume comma token
     getLexer().Lex();
 
     // Parse next operand
-    if (parseOperand(Operands, false))
+    if (parseOperand(Operands, shouldForceImediateOperand(Name, OperandIdx)))
       return true;
+
+    ++OperandIdx;
   }
 
   if (getLexer().isNot(AsmToken::EndOfStatement)) {
     SMLoc Loc = getLexer().getLoc();
     getParser().eatToEndOfStatement();
     return Error(Loc, "unexpected token");
   }
 
void RISCVAsmParser::emitLoadImm(unsigned DestReg, int64_t Value,
 
   if (Lo12)
     emitToStreamer(Out, MCInstBuilder(RISCV::ADDI)
                             .addReg(DestReg)
                             .addReg(DestReg)
                             .addImm(Lo12));
 }
 
+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)
+  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);
+
+  MCInst &AUIPC =
+      MCInstBuilder(RISCV::AUIPC).addOperand(DestReg).addExpr(Symbol);
+  emitToStreamer(Out, AUIPC);
+
+  const MCExpr *RefToLinkTmpLabel =
+      RISCVMCExpr::create(MCSymbolRefExpr::create(TmpLabel, Ctx),
+                          RISCVMCExpr::VK_RISCV_PCREL_LO, Ctx);
+
+  MCInst &ADDI = MCInstBuilder(RISCV::ADDI)
+                     .addOperand(DestReg)
+                     .addOperand(DestReg)
+                     .addExpr(RefToLinkTmpLabel);
+  emitToStreamer(Out, ADDI);
+}
+
 bool RISCVAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
                                         MCStreamer &Out) {
   Inst.setLoc(IDLoc);
 
   if (Inst.getOpcode() == 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) {
+    emitLoadLocalAddress(Inst, IDLoc, Out);
+    return false;
   }
 
   emitToStreamer(Out, Inst);
   return false;
 }
 
 extern "C" void LLVMInitializeRISCVAsmParser() {
   RegisterMCAsmParser<RISCVAsmParser> X(getTheRISCV32Target());
   RegisterMCAsmParser<RISCVAsmParser> Y(getTheRISCV64Target());
 }

llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td

 def PseudoTAILIndirect : Pseudo<(outs), (ins GPRTC:$rs1), [(Tail GPRTC:$rs1)]>,
                          PseudoInstExpansion<(JALR X0, GPR:$rs1, 0)>;
 
 def : Pat<(Tail (iPTR tglobaladdr:$dst)),
           (PseudoTAIL texternalsym:$dst)>;
 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">;
+
 /// 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)),

llvm/trunk/test/MC/RISCV/lla-invalid.s

+# RUN: not llvm-mc -triple=riscv32 < %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple=riscv64 < %s 2>&1 | FileCheck %s
+
+# Non bare symbols must be rejected
+lla a2, %lo(a_symbol) # CHECK: :[[@LINE]]:9: error: operand must be a bare symbol name
+lla a2, %hi(a_symbol) # CHECK: :[[@LINE]]:9: error: operand must be a bare symbol name

llvm/trunk/test/MC/RISCV/rvi-pseudos.s

+# RUN: llvm-mc %s -triple=riscv32 | FileCheck %s
+# RUN: llvm-mc %s -triple=riscv64 | FileCheck %s
+
+# 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: addi  a1, a1, %pcrel_lo(.Lpcrel_hi1)
+lla a1, another_symbol
+
+# Check that we can load the address of symbols that are spelled like a register
+# CHECK: .Lpcrel_hi2:
+# CHECK: auipc a2, %pcrel_hi(zero)
+# CHECK: addi  a2, a2, %pcrel_lo(.Lpcrel_hi2)
+lla a2, zero
+
+# CHECK: .Lpcrel_hi3:
+# 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