[ARM] GlobalISel: Select globals in PIC mode

[rovka]

Summary

Support the selection of G_GLOBAL_VALUE in the PIC relocation model. For
simplicity we use the same pseudoinstructions for both Darwin and ELF:
LDRLIT_ga_pcrel(_ldr). On Darwin we also use MOV_ga_pcrel(_ldr) if
available, but not on ELF (see PR28229).

This is new for ELF, so it requires a small update to the ARM pseudo
expansion pass to make sure it adds the correct constant pool modifier
and add-current-address in the case of ELF.

[peter.smith]

A potential drawback of using MOVW and MOVT in ELF is that the maximum immediate that can be used is a signed 16-bit value due to the constraint in ARMELF Section 4.6.1.1

For relocations processing MOVW and MOVT instructions (in both ARM and Thumb state), the initial addend is formed by interpreting the 16-bit literal field of the instruction as a 16-bit signed value in the range -32768 <= A < 32768. The interpretation is the same whether the relocated place contains a MOVW instruction or a MOVT instruction.

This is captured in https://bugs.llvm.org/show_bug.cgi?id=28229

I'm not familiar enough with ARMExpandPseudoInsts.cpp to know if it guarantees the addend won't overflow this range, or if you can select MOVT/MOVW only when you know the addend won't overflow? I suspect that for an overflow to occur it would require more than one global in the same section and each one accessed via a common base pointer + offset. For what its worth ARM's proprietary compiler used a separate RELA relocation section for MOVT/MOVW so that the addend could be encoded in the relocation; I don't think that this will work for llvm as not all linker's can cope with RELA relocations for ARM.

To the best of my knowledge the code and tests looks fine, but it is best if someone more familiar with the area gives it a check.

[rovka]

Thanks for the comments! I've updated the code to never use MOVT for ELF. I've also reworded the FIXME and mentioned the bug report in it.

[peter.smith]

Thanks for the update. I'm happy with the decision to not support MOVT/MOVW in ELF at this stage. I've no further comments.

[rovka]

@t.p.northover Could you have a quick look at this, please?

[t.p.northover]

Looks reasonable to me.

llvm/trunk/lib/Target/ARM/ARMExpandPseudoInsts.cpp

case ARM::tLDRLIT_ga_pcrel: {
 
       // We need a new const-pool entry to load from.
       MachineConstantPool *MCP = MBB.getParent()->getConstantPool();
       unsigned ARMPCLabelIndex = 0;
       MachineConstantPoolValue *CPV;
 
       if (IsPIC) {
         unsigned PCAdj = IsARM ? 8 : 4;
+        auto Modifier = STI->getCPModifier(GV);
         ARMPCLabelIndex = AFI->createPICLabelUId();
-        CPV = ARMConstantPoolConstant::Create(GV, ARMPCLabelIndex,
-                                              ARMCP::CPValue, PCAdj);
+        CPV = ARMConstantPoolConstant::Create(
+            GV, ARMPCLabelIndex, ARMCP::CPValue, PCAdj, Modifier,
+            /*AddCurrentAddr*/ Modifier == ARMCP::GOT_PREL);
       } else
         CPV = ARMConstantPoolConstant::Create(GV, ARMCP::no_modifier);
 
       MachineInstrBuilder MIB =
           BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(LDRLITOpc), DstReg)
             .addConstantPoolIndex(MCP->getConstantPoolIndex(CPV, 4));
       if (IsARM)
         MIB.addImm(0);

llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp

 }
 
 bool ARMInstructionSelector::selectGlobal(MachineInstrBuilder &MIB,
                                           MachineRegisterInfo &MRI) const {
   if (TII.getSubtarget().isROPI() || TII.getSubtarget().isRWPI()) {
     DEBUG(dbgs() << "ROPI and RWPI not supported yet\n");
     return false;
   }
-  if (TM.isPositionIndependent()) {
-    DEBUG(dbgs() << "PIC not supported yet\n");
-    return false;
-  }
 
   auto GV = MIB->getOperand(1).getGlobal();
   if (GV->isThreadLocal()) {
     DEBUG(dbgs() << "TLS variables not supported yet\n");
     return false;
   }
 
   auto &MBB = *MIB->getParent();
   auto &MF = *MBB.getParent();
 
   auto ObjectFormat = TII.getSubtarget().getTargetTriple().getObjectFormat();
   bool UseMovt = TII.getSubtarget().useMovt(MF);
 
+  unsigned Alignment = 4;
+  if (TM.isPositionIndependent()) {
+    bool Indirect = TII.getSubtarget().isGVIndirectSymbol(GV);
+    // FIXME: Taking advantage of MOVT for ELF is pretty involved, so we don't
+    // support it yet. See PR28229.
+    unsigned Opc =
+        UseMovt && !TII.getSubtarget().isTargetELF()
+            ? (Indirect ? ARM::MOV_ga_pcrel_ldr : ARM::MOV_ga_pcrel)
+            : (Indirect ? ARM::LDRLIT_ga_pcrel_ldr : ARM::LDRLIT_ga_pcrel);
+    MIB->setDesc(TII.get(Opc));
+
+    if (TII.getSubtarget().isTargetDarwin())
+      MIB->getOperand(1).setTargetFlags(ARMII::MO_NONLAZY);
+
+    if (Indirect)
+      MIB.addMemOperand(MF.getMachineMemOperand(
+          MachinePointerInfo::getGOT(MF), MachineMemOperand::MOLoad,
+          TM.getPointerSize(), Alignment));
+
+    return true;
+  }
+
   if (ObjectFormat == Triple::ELF) {
     if (UseMovt) {
       MIB->setDesc(TII.get(ARM::MOVi32imm));
     } else {
       // Load the global's address from the constant pool.
       MIB->setDesc(TII.get(ARM::LDRi12));
       MIB->RemoveOperand(1);
-      unsigned Alignment = 4;
       MIB.addConstantPoolIndex(
              MF.getConstantPool()->getConstantPoolIndex(GV, Alignment),
              /* Offset */ 0, /* TargetFlags */ 0)
           .addMemOperand(MF.getMachineMemOperand(
               MachinePointerInfo::getConstantPool(MF),
               MachineMemOperand::MOLoad, TM.getPointerSize(), Alignment))
           .addImm(0)
           .add(predOps(ARMCC::AL));

llvm/trunk/lib/Target/ARM/ARMSubtarget.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_ARM_ARMSUBTARGET_H
 #define LLVM_LIB_TARGET_ARM_ARMSUBTARGET_H
 
 #include "ARMBaseInstrInfo.h"
 #include "ARMBaseRegisterInfo.h"
+#include "ARMConstantPoolValue.h"
 #include "ARMFrameLowering.h"
 #include "ARMISelLowering.h"
 #include "ARMSelectionDAGInfo.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/CodeGen/GlobalISel/CallLowering.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
 #include "llvm/CodeGen/GlobalISel/RegisterBankInfo.h"
public:
 
   int getPreISelOperandLatencyAdjustment() const {
     return PreISelOperandLatencyAdjustment;
   }
 
   /// True if the GV will be accessed via an indirect symbol.
   bool isGVIndirectSymbol(const GlobalValue *GV) const;
 
+  /// Returns the constant pool modifier needed to access the GV.
+  ARMCP::ARMCPModifier getCPModifier(const GlobalValue *GV) const;
+
   /// True if fast-isel is used.
   bool useFastISel() const;
 
   /// Returns the correct return opcode for the current feature set.
   /// Use BX if available to allow mixing thumb/arm code, but fall back
   /// to plain mov pc,lr on ARMv4.
   unsigned getReturnOpcode() const {
     if (isThumb())

llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp

bool ARMSubtarget::isGVIndirectSymbol(const GlobalValue *GV) const {
   // for GVs that are known to be local to the dso.
   if (isTargetMachO() && TM.isPositionIndependent() &&
       (GV->isDeclarationForLinker() || GV->hasCommonLinkage()))
     return true;
 
   return false;
 }
 
+ARMCP::ARMCPModifier ARMSubtarget::getCPModifier(const GlobalValue *GV) const {
+  if (isTargetELF() && TM.isPositionIndependent() &&
+      !TM.shouldAssumeDSOLocal(*GV->getParent(), GV))
+    return ARMCP::GOT_PREL;
+  return ARMCP::no_modifier;
+}
+
 unsigned ARMSubtarget::getMispredictionPenalty() const {
   return SchedModel.MispredictPenalty;
 }
 
 bool ARMSubtarget::hasSinCos() const {
   return isTargetWatchOS() ||
     (isTargetIOS() && !getTargetTriple().isOSVersionLT(7, 0));
 }

llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-isel-globals-pic.ll

+; RUN: llc -mtriple armv7-linux -relocation-model=pic -global-isel %s -o - | FileCheck %s -check-prefixes=CHECK,ELF
+; RUN: llc -mtriple armv7-linux -relocation-model=pic -mattr=+no-movt -global-isel %s -o - | FileCheck %s -check-prefixes=CHECK,ELF
+; RUN: llc -mtriple armv7-darwin -relocation-model=pic -global-isel %s -o - | FileCheck %s -check-prefixes=CHECK,DARWIN,DARWIN-MOVT
+; RUN: llc -mtriple armv7-darwin -relocation-model=pic -mattr=+no-movt -global-isel %s -o - | FileCheck %s -check-prefixes=CHECK,DARWIN,DARWIN-NOMOVT
+
+@internal_global = internal global i32 42
+define i32 @test_internal_global() {
+; CHECK-LABEL: test_internal_global:
+; ELF: ldr [[OFFSET:r[0-9]+]], [[LABEL:.L[[:alnum:]_]+]]
+; ELF: [[ANCHOR:.L[[:alnum:]_]+]]:
+; DARWIN-NOMOVT: ldr [[OFFSET:r[0-9]+]], [[LABEL:L[[:alnum:]_]+]]
+; DARWIN-MOVT: movw [[OFFSET:r[0-9]+]], :lower16:(_internal_global-([[ANCHOR:L[[:alnum:]_]+]]+8))
+; DARWIN-MOVT-NEXT: movt [[OFFSET]], :upper16:(_internal_global-([[ANCHOR]]+8))
+; DARWIN: [[ANCHOR:L[[:alnum:]_]+]]:
+; CHECK-NEXT: add r[[ADDR:[0-9]+]], pc, [[OFFSET]]
+; CHECK-NEXT: ldr r0, [r[[ADDR]]]
+; CHECK-NEXT: bx lr
+; ELF: [[LABEL]]:
+; ELF-NEXT: .long internal_global-([[ANCHOR]]+8)
+; DARWIN-NOMOVT: [[LABEL]]:
+; DARWIN-NOMOVT-NEXT: .long _internal_global-([[ANCHOR]]+8)
+; DARWIN-MOVT-NOT: .long _internal_global
+
+entry:
+  %v = load i32, i32* @internal_global
+  ret i32 %v
+}
+
+@external_global = external global i32
+define i32 @test_external_global() {
+; CHECK-LABEL: test_external_global:
+; ELF: ldr [[OFFSET:r[0-9]+]], [[LABEL:.L[[:alnum:]_]+]]
+; ELF: [[ANCHOR:.L[[:alnum:]_]+]]:
+; ELF-NEXT: ldr r[[ADDR:[0-9]+]], [pc, [[OFFSET]]]
+; DARWIN-NOMOVT: ldr [[OFFSET:r[0-9]+]], [[LABEL:L[[:alnum:]_]+]]
+; DARWIN-MOVT: movw [[OFFSET:r[0-9]+]], :lower16:(L_external_global$non_lazy_ptr-([[ANCHOR:L[[:alnum:]_]+]]+8))
+; DARWIN-MOVT: movt [[OFFSET]], :upper16:(L_external_global$non_lazy_ptr-([[ANCHOR]]+8))
+; DARWIN: [[ANCHOR:L[[:alnum:]_]+]]:
+; DARWIN: ldr r[[ADDR:[0-9]+]], [pc, [[OFFSET]]]
+; CHECK-NEXT: ldr r0, [r[[ADDR]]]
+; CHECK-NEXT: bx lr
+; ELF: [[LABEL]]:
+; ELF: [[TMPLABEL:.L[[:alnum:]_]+]]:
+; ELF: .long external_global(GOT_PREL)-(([[ANCHOR]]+8)-[[TMPLABEL]])
+; DARWIN-NOMOVT: [[LABEL]]:
+; DARWIN-NOMOVT: .long L_external_global$non_lazy_ptr-([[ANCHOR]]+8)
+; DARWIN-NOMOVT-NOT: .long L_external_global
+entry:
+  %v = load i32, i32* @external_global
+  ret i32 %v
+}
+
+; ELF: internal_global:
+; DARWIN: _internal_global:
+; CHECK: .long 42
+; ELF: .size internal_global, 4
+; DARWIN: L_external_global$non_lazy_ptr:
+; DARWIN: .indirect_symbol _external_global

llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-select-globals-pic.mir

+# RUN: llc -O0 -mtriple arm-linux -relocation-model=pic -mattr=+no-movt -global-isel -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s -check-prefixes=CHECK,ELF
+# RUN: llc -O0 -mtriple arm-linux -relocation-model=pic -mattr=-no-movt,+v8m -global-isel -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s -check-prefixes=CHECK,ELF
+# RUN: llc -O0 -mtriple arm-darwin -relocation-model=pic -mattr=+no-movt -global-isel -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s -check-prefixes=CHECK,DARWIN-NOMOVT
+# RUN: llc -O0 -mtriple arm-darwin -relocation-model=pic -mattr=-no-movt,+v8m -global-isel -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s -check-prefixes=CHECK,DARWIN-MOVT
+--- |
+  @internal_global = internal global i32 42
+  define void @test_internal_global() { ret void }
+
+  @external_global = external global i32
+  define void @test_external_global() { ret void }
+...
+---
+name:            test_internal_global
+# CHECK-LABEL: name: test_internal_global
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: gprb }
+  - { id: 1, class: gprb }
+body:             |
+  bb.0:
+    %0(p0) = G_GLOBAL_VALUE @internal_global
+    ; DARWIN-MOVT: [[G:%[0-9]+]] = MOV_ga_pcrel {{.*}}@internal_global
+    ; DARWIN-NOMOVT: [[G:%[0-9]+]] = LDRLIT_ga_pcrel {{.*}}@internal_global
+    ; ELF: [[G:%[0-9]+]] = LDRLIT_ga_pcrel {{.*}}@internal_global
+
+    %1(s32) = G_LOAD %0(p0) :: (load 4 from @internal_global)
+    ; CHECK: [[V:%[0-9]+]] = LDRi12 [[G]], 0, 14, _ :: (load 4 from @internal_global)
+
+    %r0 = COPY %1(s32)
+    ; CHECK: %r0 = COPY [[V]]
+
+    BX_RET 14, _, implicit %r0
+    ; CHECK: BX_RET 14, _, implicit %r0
+...
+---
+name:            test_external_global
+# CHECK-LABEL: name: test_external_global
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: gprb }
+  - { id: 1, class: gprb }
+body:             |
+  bb.0:
+    %0(p0) = G_GLOBAL_VALUE @external_global
+    ; DARWIN-MOVT: [[G:%[0-9]+]] = MOV_ga_pcrel_ldr {{.*}} @external_global :: (load 4 from got)
+    ; DARWIN-NOMOVT: [[G:%[0-9]+]] = LDRLIT_ga_pcrel_ldr {{.*}}@external_global :: (load 4 from got)
+    ; ELF: [[G:%[0-9]+]] = LDRLIT_ga_pcrel_ldr @external_global :: (load 4 from got)
+
+    %1(s32) = G_LOAD %0(p0) :: (load 4 from @external_global)
+    ; CHECK: [[V:%[0-9]+]] = LDRi12 [[G]], 0, 14, _ :: (load 4 from @external_global)
+
+    %r0 = COPY %1(s32)
+    ; CHECK: %r0 = COPY [[V]]
+
+    BX_RET 14, _, implicit %r0
+    ; CHECK: BX_RET 14, _, implicit %r0
+...

llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-unsupported.ll

; ROPI-RWPI-LABEL: warning: Instruction selection used fallback path for test_thread_local_global
   %v = load i32, i32* @thread_local_global
   ret i32 %v
 }
 
 @a_global = external global i32
 
 define i32 @test_global_reloc_models() {
 ; This is only unsupported for the PIC, ROPI, RWPI relocation modes.
-; PIC: remark: {{.*}} cannot select: {{.*}} G_GLOBAL_VALUE
-; PIC-LABEL: warning: Instruction selection used fallback path for test_global_reloc_models
 ; ROPI: remark: {{.*}} cannot select: {{.*}} G_GLOBAL_VALUE
 ; ROPI-LABEL: warning: Instruction selection used fallback path for test_global_reloc_models
 ; RWPI: remark: {{.*}} cannot select: {{.*}} G_GLOBAL_VALUE
 ; RWPI-LABEL: warning: Instruction selection used fallback path for test_global_reloc_models
 ; ROPI-RWPI: remark: {{.*}} cannot select: {{.*}} G_GLOBAL_VALUE
 ; ROPI-RWPI-LABEL: warning: Instruction selection used fallback path for test_global_reloc_models
   %v = load i32, i32* @a_global
   ret i32 %v
 }
 
 attributes #0 = { "target-features"="+thumb-mode" }