[RISCV] Add lowering of addressing sequences for PIC

[lewis-revill]

Summary

This patch allows lowering of PIC addresses by using PC-relative addressing for DSO-local symbols and accessing the address through the global offset table for non-DSO-local symbols.

Builds on D54143

[jrtc27]

Half of this (the MC side of the GOT modifier) is already proposed in D55279. Also note that the assembly modifier is not %got_hi but %got_pcrel_hi after a discussion with upstream (check the assembler manual) to clearly convey that it's a PC-relative relocation to the GOT entry rather than generating a GOT index as would be the case on some older architectures. I did however, like you, use GOT_HI in the various LLVM-internal enumeration entries to match the name of the relocation.

[lewis-revill]

That's an interesting coincidence! I think I'll rebase this patch on top of that patch since it seems likely to make it upstream.

[lewis-revill]

It's probably worth noting that the %pcrel_lo relocations to the label appear to be evaluated to a constant when -mattr=+relax is not provided... I remember someone said that this was going to be made default? If so I think it would be good to wait for that before this is commited.

[lewis-revill]

Update to use the MC layer changes in D55279. There are still some MC layer changes left in this patch waiting for D55279 to be updated.

[jrtc27]

In D55303#1320326, @lewis-revill wrote:

It's probably worth noting that the %pcrel_lo relocations to the label appear to be evaluated to a constant when -mattr=+relax is not provided... I remember someone said that this was going to be made default? If so I think it would be good to wait for that before this is commited.

Ah, that would be because D54029 isn't yet merged.

[lewis-revill]

Rebased with updated dependency.

[lewis-revill]

Rebasing with latest changes to D55279 and modifying this patch to use and expand PseudoLA (D55325).

[lewis-revill]

Rebased and updated to use and expand PseudoLA.

[jrtc27]

Almost there now from my point of view, but we'll see what the others say (especially Alex).

lib/Target/RISCV/RISCVISelLowering.cpp

353 ↗

(On Diff #177650)

Can we combine this with getAddr, pushing the common isPositionIndependent() check inside getAddr? Then the UseGOT flag becomes NonLocal or similar (or alternatively invert it and make it an IsLocal flag), and happens to only influence code generation for PIC. This should simplify all its uses.

[jrtc27]

lib/Target/RISCV/RISCVISelLowering.h

116 ↗

(On Diff #177650)

Flags is never provided so it's always 0; can we drop it?

[lewis-revill]

Remove unnecessary Flags operand

[lewis-revill]

lib/Target/RISCV/RISCVISelLowering.cpp

353 ↗

(On Diff #177650)

I'm not sure this is a good idea? We'd still have to check in lowerGlobalAddress for GOT, and I feel leaving getAddr to check isPositionIndependent seems misleading, and the other option is to end up checking isPositionIndependent twice for this path, and gaining nothing in the process. It's certainly doable though, if people think it is clearer.

[jrtc27]

lib/Target/RISCV/RISCVISelLowering.cpp

353 ↗

(On Diff #177650)

No you don’t, all you need to do is check shouldAssumeDSOLocal and pass that to getAddr as an IsLocal flag. The fact that the flag is only actually used on the PIC branch of getAddr is an implementation detail that lowerGlobalAddress doesn’t need to know about.

[lewis-revill]

Move getAddrPIC() and isPositionIndependent() checks to getAddr()

[lewis-revill]

lib/Target/RISCV/RISCVISelLowering.cpp

353 ↗

(On Diff #177650)

I tried this and it did look cleaner so I implemented it. Thanks for the suggestion.

[rogfer01]

Minor comment above but other than that this looks good to me too.

lib/Target/RISCV/RISCVExpandPseudoInsts.cpp

616 ↗

(On Diff #178089)

Perhaps it is clearer if we call expandLoadLocalAddress here instead of repeating the call to expandAuipcInstPair.

[lewis-revill]

Rebased and updated comments.

[jrtc27]

One minor comment, but otherwise I think this is ready to land now?

lib/Target/RISCV/RISCVExpandPseudoInsts.cpp

616 ↗

(On Diff #178089)

I think it's better to match RISCVAsmParser::emitLoadAddress, which would make this something like:

unsigned SecondOpcode;
unsigned FlagsHi;
if (MF->getTarget().isPositionIndependent()) {
  const auto &STI = MF->getSubtarget<RISCVSubtarget>();
  SecondOpcode = STI.is64Bit() ? RISCV::LD : RISCV::LW;
  FlagsHi = RISCVII::MO_GOT_HI;
} else {
  SecondOpcode = RISCV::ADDI;
  FlagsHi = RISCVII::MO_PCREL_HI,
}
return expandAuipcInstPair(MBB, MBBI, NextMBBI, FlagsHi,
                           SecondOpcode);

[lewis-revill]

Rebased and modified expandLoadAddress.

[asb]

Please add RV64 RUN lines to pic-models.ll and add nounwind attributes, otherwise this looks good to me. Thanks!

llvm/trunk/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp

bool expandAtomicBinOp(MachineBasicBlock &MBB,
                          MachineBasicBlock::iterator &NextMBBI);
   bool expandAtomicMinMaxOp(MachineBasicBlock &MBB,
                             MachineBasicBlock::iterator MBBI,
                             AtomicRMWInst::BinOp, bool IsMasked, int Width,
                             MachineBasicBlock::iterator &NextMBBI);
   bool expandAtomicCmpXchg(MachineBasicBlock &MBB,
                            MachineBasicBlock::iterator MBBI, bool IsMasked,
                            int Width, MachineBasicBlock::iterator &NextMBBI);
+  bool expandAuipcInstPair(MachineBasicBlock &MBB,
+                           MachineBasicBlock::iterator MBBI,
+                           MachineBasicBlock::iterator &NextMBBI,
+                           unsigned FlagsHi, unsigned SecondOpcode);
   bool expandLoadLocalAddress(MachineBasicBlock &MBB,
                               MachineBasicBlock::iterator MBBI,
                               MachineBasicBlock::iterator &NextMBBI);
+  bool expandLoadAddress(MachineBasicBlock &MBB,
+                         MachineBasicBlock::iterator MBBI,
+                         MachineBasicBlock::iterator &NextMBBI);
 };
 
 char RISCVExpandPseudo::ID = 0;
 
 bool RISCVExpandPseudo::runOnMachineFunction(MachineFunction &MF) {
   TII = static_cast<const RISCVInstrInfo *>(MF.getSubtarget().getInstrInfo());
   bool Modified = false;
   for (auto &MBB : MF)
bool RISCVExpandPseudo::expandMI(MachineBasicBlock &MBB,
   case RISCV::PseudoCmpXchg32:
     return expandAtomicCmpXchg(MBB, MBBI, false, 32, NextMBBI);
   case RISCV::PseudoCmpXchg64:
     return expandAtomicCmpXchg(MBB, MBBI, false, 64, NextMBBI);
   case RISCV::PseudoMaskedCmpXchg32:
     return expandAtomicCmpXchg(MBB, MBBI, true, 32, NextMBBI);
   case RISCV::PseudoLLA:
     return expandLoadLocalAddress(MBB, MBBI, NextMBBI);
+  case RISCV::PseudoLA:
+    return expandLoadAddress(MBB, MBBI, NextMBBI);
   }
 
   return false;
 }
 
 static unsigned getLRForRMW32(AtomicOrdering Ordering) {
   switch (Ordering) {
   default:
bool RISCVExpandPseudo::expandAtomicCmpXchg(
   LivePhysRegs LiveRegs;
   computeAndAddLiveIns(LiveRegs, *LoopHeadMBB);
   computeAndAddLiveIns(LiveRegs, *LoopTailMBB);
   computeAndAddLiveIns(LiveRegs, *DoneMBB);
 
   return true;
 }
 
-bool RISCVExpandPseudo::expandLoadLocalAddress(
+bool RISCVExpandPseudo::expandAuipcInstPair(
     MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
-    MachineBasicBlock::iterator &NextMBBI) {
+    MachineBasicBlock::iterator &NextMBBI, unsigned FlagsHi,
+    unsigned SecondOpcode) {
   MachineFunction *MF = MBB.getParent();
   MachineInstr &MI = *MBBI;
   DebugLoc DL = MI.getDebugLoc();
 
   unsigned DestReg = MI.getOperand(0).getReg();
   const MachineOperand &Symbol = MI.getOperand(1);
 
   MachineBasicBlock *NewMBB = MF->CreateMachineBasicBlock(MBB.getBasicBlock());
 
   // Tell AsmPrinter that we unconditionally want the symbol of this label to be
   // emitted.
   NewMBB->setLabelMustBeEmitted();
 
   MF->insert(++MBB.getIterator(), NewMBB);
 
   BuildMI(NewMBB, DL, TII->get(RISCV::AUIPC), DestReg)
-      .addDisp(Symbol, 0, RISCVII::MO_PCREL_HI);
-  BuildMI(NewMBB, DL, TII->get(RISCV::ADDI), DestReg)
+      .addDisp(Symbol, 0, FlagsHi);
+  BuildMI(NewMBB, DL, TII->get(SecondOpcode), DestReg)
       .addReg(DestReg)
       .addMBB(NewMBB, RISCVII::MO_PCREL_LO);
 
   // Move all the rest of the instructions to NewMBB.
   NewMBB->splice(NewMBB->end(), &MBB, std::next(MBBI), MBB.end());
   // Update machine-CFG edges.
   NewMBB->transferSuccessorsAndUpdatePHIs(&MBB);
   // Make the original basic block fall-through to the new.
   MBB.addSuccessor(NewMBB);
 
   // Make sure live-ins are correctly attached to this new basic block.
   LivePhysRegs LiveRegs;
   computeAndAddLiveIns(LiveRegs, *NewMBB);
 
   NextMBBI = MBB.end();
   MI.eraseFromParent();
   return true;
 }
 
+bool RISCVExpandPseudo::expandLoadLocalAddress(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+    MachineBasicBlock::iterator &NextMBBI) {
+  return expandAuipcInstPair(MBB, MBBI, NextMBBI, RISCVII::MO_PCREL_HI,
+                             RISCV::ADDI);
+}
+
+bool RISCVExpandPseudo::expandLoadAddress(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+    MachineBasicBlock::iterator &NextMBBI) {
+  MachineFunction *MF = MBB.getParent();
+
+  unsigned SecondOpcode;
+  unsigned FlagsHi;
+  if (MF->getTarget().isPositionIndependent()) {
+    const auto &STI = MF->getSubtarget<RISCVSubtarget>();
+    SecondOpcode = STI.is64Bit() ? RISCV::LD : RISCV::LW;
+    FlagsHi = RISCVII::MO_GOT_HI;
+  } else {
+    SecondOpcode = RISCV::ADDI;
+    FlagsHi = RISCVII::MO_PCREL_HI;
+  }
+  return expandAuipcInstPair(MBB, MBBI, NextMBBI, FlagsHi, SecondOpcode);
+}
+
 } // end of anonymous namespace
 
 INITIALIZE_PASS(RISCVExpandPseudo, "riscv-expand-pseudo",
                 RISCV_EXPAND_PSEUDO_NAME, false, false)
 namespace llvm {
 
 FunctionPass *createRISCVExpandPseudoPass() { return new RISCVExpandPseudo(); }
 
 } // end of namespace llvm

llvm/trunk/lib/Target/RISCV/RISCVISelLowering.h

private:
   SDValue LowerCall(TargetLowering::CallLoweringInfo &CLI,
                     SmallVectorImpl<SDValue> &InVals) const override;
   bool shouldConvertConstantLoadToIntImm(const APInt &Imm,
                                          Type *Ty) const override {
     return true;
   }
 
   template <class NodeTy>
-  SDValue getAddr(NodeTy *N, SelectionDAG &DAG) const;
+  SDValue getAddr(NodeTy *N, SelectionDAG &DAG, bool IsLocal = true) const;
 
   SDValue lowerGlobalAddress(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerBlockAddress(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerConstantPool(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerSELECT(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerVASTART(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerFRAMEADDR(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;

llvm/trunk/lib/Target/RISCV/RISCVISelLowering.cpp

 
 static SDValue getTargetNode(ConstantPoolSDNode *N, SDLoc DL, EVT Ty,
                              SelectionDAG &DAG, unsigned Flags) {
   return DAG.getTargetConstantPool(N->getConstVal(), Ty, N->getAlignment(),
                                    N->getOffset(), Flags);
 }
 
 template <class NodeTy>
-SDValue RISCVTargetLowering::getAddr(NodeTy *N, SelectionDAG &DAG) const {
+SDValue RISCVTargetLowering::getAddr(NodeTy *N, SelectionDAG &DAG,
+                                     bool IsLocal) const {
   SDLoc DL(N);
   EVT Ty = getPointerTy(DAG.getDataLayout());
 
+  if (isPositionIndependent()) {
+    SDValue Addr = getTargetNode(N, DL, Ty, DAG, 0);
+    if (IsLocal)
+      // Use PC-relative addressing to access the symbol. This generates the
+      // pattern (PseudoLLA sym), which expands to (addi (auipc %pcrel_hi(sym))
+      // %pcrel_lo(auipc)).
+      return SDValue(DAG.getMachineNode(RISCV::PseudoLLA, DL, Ty, Addr), 0);
+
+    // Use PC-relative addressing to access the GOT for this symbol, then load
+    // the address from the GOT. This generates the pattern (PseudoLA sym),
+    // which expands to (ld (addi (auipc %got_pcrel_hi(sym)) %pcrel_lo(auipc))).
+    return SDValue(DAG.getMachineNode(RISCV::PseudoLA, DL, Ty, Addr), 0);
+  }
+
   switch (getTargetMachine().getCodeModel()) {
   default:
     report_fatal_error("Unsupported code model for lowering");
   case CodeModel::Small: {
     // Generate a sequence for accessing addresses within the first 2 GiB of
     // address space. This generates the pattern (addi (lui %hi(sym)) %lo(sym)).
     SDValue AddrHi = getTargetNode(N, DL, Ty, DAG, RISCVII::MO_HI);
     SDValue AddrLo = getTargetNode(N, DL, Ty, DAG, RISCVII::MO_LO);

 SDValue RISCVTargetLowering::lowerGlobalAddress(SDValue Op,
                                                 SelectionDAG &DAG) const {
   SDLoc DL(Op);
   EVT Ty = Op.getValueType();
   GlobalAddressSDNode *N = cast<GlobalAddressSDNode>(Op);
   int64_t Offset = N->getOffset();
   MVT XLenVT = Subtarget.getXLenVT();
 
-  if (isPositionIndependent())
-    report_fatal_error("Unable to lowerGlobalAddress");
-
-  SDValue Addr = getAddr(N, DAG);
+  const GlobalValue *GV = N->getGlobal();
+  bool IsLocal = getTargetMachine().shouldAssumeDSOLocal(*GV->getParent(), GV);
+  SDValue Addr = getAddr(N, DAG, IsLocal);
 
   // In order to maximise the opportunity for common subexpression elimination,
   // emit a separate ADD node for the global address offset instead of folding
   // it in the global address node. Later peephole optimisations may choose to
   // fold it back in when profitable.
   if (Offset != 0)
     return DAG.getNode(ISD::ADD, DL, Ty, Addr,
                        DAG.getConstant(Offset, DL, XLenVT));
   return Addr;
 }
 
 SDValue RISCVTargetLowering::lowerBlockAddress(SDValue Op,
                                                SelectionDAG &DAG) const {
   BlockAddressSDNode *N = cast<BlockAddressSDNode>(Op);
 
-  if (isPositionIndependent())
-    report_fatal_error("Unable to lowerBlockAddress");
-
   return getAddr(N, DAG);
 }
 
 SDValue RISCVTargetLowering::lowerConstantPool(SDValue Op,
                                                SelectionDAG &DAG) const {
   ConstantPoolSDNode *N = cast<ConstantPoolSDNode>(Op);
 
-  if (isPositionIndependent())
-    report_fatal_error("Unable to lowerConstantPool");
-
   return getAddr(N, DAG);
 }
 
 SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
   SDValue CondV = Op.getOperand(0);
   SDValue TrueV = Op.getOperand(1);
   SDValue FalseV = Op.getOperand(2);
   SDLoc DL(Op);

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

unsigned RISCVInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
   case TargetOpcode::EH_LABEL:
   case TargetOpcode::IMPLICIT_DEF:
   case TargetOpcode::KILL:
   case TargetOpcode::DBG_VALUE:
     return 0;
   case RISCV::PseudoCALL:
   case RISCV::PseudoTAIL:
   case RISCV::PseudoLLA:
+  case RISCV::PseudoLA:
     return 8;
   case TargetOpcode::INLINEASM:
   case TargetOpcode::INLINEASM_BR: {
     const MachineFunction &MF = *MI.getParent()->getParent();
     const auto &TM = static_cast<const RISCVTargetMachine &>(MF.getTarget());
     return getInlineAsmLength(MI.getOperand(0).getSymbolName(),
                               *TM.getMCAsmInfo());
   }

llvm/trunk/lib/Target/RISCV/RISCVMCInstLower.cpp

case RISCVII::MO_HI:
     Kind = RISCVMCExpr::VK_RISCV_HI;
     break;
   case RISCVII::MO_PCREL_LO:
     Kind = RISCVMCExpr::VK_RISCV_PCREL_LO;
     break;
   case RISCVII::MO_PCREL_HI:
     Kind = RISCVMCExpr::VK_RISCV_PCREL_HI;
     break;
+  case RISCVII::MO_GOT_HI:
+    Kind = RISCVMCExpr::VK_RISCV_GOT_HI;
+    break;
   }
 
   const MCExpr *ME =
       MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, Ctx);
 
   if (!MO.isJTI() && !MO.isMBB() && MO.getOffset())
     ME = MCBinaryExpr::createAdd(
         ME, MCConstantExpr::create(MO.getOffset(), Ctx), Ctx);

llvm/trunk/lib/Target/RISCV/Utils/RISCVBaseInfo.h

 
 enum {
   MO_None,
   MO_CALL,
   MO_LO,
   MO_HI,
   MO_PCREL_LO,
   MO_PCREL_HI,
+  MO_GOT_HI,
 };
 } // namespace RISCVII
 
 // Describes the predecessor/successor bits used in the FENCE instruction.
 namespace RISCVFenceField {
 enum FenceField {
   I = 8,
   O = 4,

llvm/trunk/test/CodeGen/RISCV/pic-models.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -relocation-model=static < %s \
+; RUN:     | FileCheck -check-prefix=RV32-STATIC %s
+; RUN: llc -mtriple=riscv32 -relocation-model=pic < %s \
+; RUN:     | FileCheck -check-prefix=RV32-PIC %s
+; RUN: llc -mtriple=riscv64 -relocation-model=static < %s \
+; RUN:     | FileCheck -check-prefix=RV64-STATIC %s
+; RUN: llc -mtriple=riscv64 -relocation-model=pic < %s \
+; RUN:     | FileCheck -check-prefix=RV64-PIC %s
+
+; Check basic lowering of PIC addressing.
+; TODO: Check other relocation models?
+
+@external_var = external global i32
+@internal_var = internal global i32 42
+
+
+; external address
+
+define i32* @f1() nounwind {
+; RV32-STATIC-LABEL: f1:
+; RV32-STATIC:       # %bb.0: # %entry
+; RV32-STATIC-NEXT:    lui a0, %hi(external_var)
+; RV32-STATIC-NEXT:    addi a0, a0, %lo(external_var)
+; RV32-STATIC-NEXT:    ret
+;
+; RV32-PIC-LABEL: f1:
+; RV32-PIC:       # %bb.0: # %entry
+; RV32-PIC-NEXT:  .LBB0_1: # %entry
+; RV32-PIC-NEXT:    # Label of block must be emitted
+; RV32-PIC-NEXT:    auipc a0, %got_pcrel_hi(external_var)
+; RV32-PIC-NEXT:    lw a0, %pcrel_lo(.LBB0_1)(a0)
+; RV32-PIC-NEXT:    ret
+;
+; RV64-STATIC-LABEL: f1:
+; RV64-STATIC:       # %bb.0: # %entry
+; RV64-STATIC-NEXT:    lui a0, %hi(external_var)
+; RV64-STATIC-NEXT:    addi a0, a0, %lo(external_var)
+; RV64-STATIC-NEXT:    ret
+;
+; RV64-PIC-LABEL: f1:
+; RV64-PIC:       # %bb.0: # %entry
+; RV64-PIC-NEXT:  .LBB0_1: # %entry
+; RV64-PIC-NEXT:    # Label of block must be emitted
+; RV64-PIC-NEXT:    auipc a0, %got_pcrel_hi(external_var)
+; RV64-PIC-NEXT:    ld a0, %pcrel_lo(.LBB0_1)(a0)
+; RV64-PIC-NEXT:    ret
+entry:
+  ret i32* @external_var
+}
+
+
+; internal address
+
+define i32* @f2() nounwind {
+; RV32-STATIC-LABEL: f2:
+; RV32-STATIC:       # %bb.0: # %entry
+; RV32-STATIC-NEXT:    lui a0, %hi(internal_var)
+; RV32-STATIC-NEXT:    addi a0, a0, %lo(internal_var)
+; RV32-STATIC-NEXT:    ret
+;
+; RV32-PIC-LABEL: f2:
+; RV32-PIC:       # %bb.0: # %entry
+; RV32-PIC-NEXT:  .LBB1_1: # %entry
+; RV32-PIC-NEXT:    # Label of block must be emitted
+; RV32-PIC-NEXT:    auipc a0, %pcrel_hi(internal_var)
+; RV32-PIC-NEXT:    addi a0, a0, %pcrel_lo(.LBB1_1)
+; RV32-PIC-NEXT:    ret
+;
+; RV64-STATIC-LABEL: f2:
+; RV64-STATIC:       # %bb.0: # %entry
+; RV64-STATIC-NEXT:    lui a0, %hi(internal_var)
+; RV64-STATIC-NEXT:    addi a0, a0, %lo(internal_var)
+; RV64-STATIC-NEXT:    ret
+;
+; RV64-PIC-LABEL: f2:
+; RV64-PIC:       # %bb.0: # %entry
+; RV64-PIC-NEXT:  .LBB1_1: # %entry
+; RV64-PIC-NEXT:    # Label of block must be emitted
+; RV64-PIC-NEXT:    auipc a0, %pcrel_hi(internal_var)
+; RV64-PIC-NEXT:    addi a0, a0, %pcrel_lo(.LBB1_1)
+; RV64-PIC-NEXT:    ret
+entry:
+  ret i32* @internal_var
+}