Fix and test inter-procedural register allocation for ARM

[ostannard]

Summary

• Avoid a crash when IPRA calls ARMFrameLowering::determineCalleeSaves with a null RegScavenger. Simply not updating the register scavenger is fine because IPRA only cares about the SavedRegs vector, the acutal code of the function has already been generated at this point.
• Add a new hook to TargetRegisterInfo to get the set of registers which can be clobbered inside a call, even if the compiler can see both sides, by linker-generated code.

[arsenm]

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

493

Why aren’t these explicit clobbers on the call itself?

[ostannard]

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

493

This is a property of the calling convention, not the BL instruction, so I think it makes more sense to have it here. I think we also use BL as a long branch inside Thumb1 functions, in which case it clobbers lr but nor r12.

[ostannard]

Ping

[ostannard]

Ping.

[dmgreen]

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

492

*which

496

This returns a Null terminated list (or null), in the same way as getCalleeSavedRegs above? I guess that's Ok.

An arrayref might be a better interface for it, all the same.

[ostannard]

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

496

Yes, it matches the functions above, but I suppose an ArrayRef would be a better interface, so I'll change it.

[ostannard]

• Switch to using ArrayRef instead of null-terminated array

[dmgreen]

LGTM. I think this is a sensible way to deal with r12. It is a bit special.

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

495

Nit: is it worth making an ArrayRef const?

[mgorny]

This change broke NetBSD buildbot:
http://lab.llvm.org:8011/builders/netbsd-amd64/builds/21240/steps/ninja%20build%20local/logs/stdio

Please fix.

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

virtual const uint32_t *getCallPreservedMask(const MachineFunction &MF,
     return nullptr;
   }
 
   /// Return a register mask that clobbers everything.
   virtual const uint32_t *getNoPreservedMask() const {
     llvm_unreachable("target does not provide no preserved mask");
   }
 
+  /// Return a null-terminated list of all of the registers thich are clobbered
+  /// "inside" a call to the given function. For example, these might be needed
+  /// for PLT sequences of long-branch veneers.
+  virtual const MCPhysReg *
+  getIntraCallClobberedRegs(const MachineFunction *MF) const {
+    return nullptr;
+  }
+
   /// Return true if all bits that are set in mask \p mask0 are also set in
   /// \p mask1.
   bool regmaskSubsetEqual(const uint32_t *mask0, const uint32_t *mask1) const;
 
   /// Return all the call-preserved register masks defined for this target.
   virtual ArrayRef<const uint32_t *> getRegMasks() const = 0;
   virtual ArrayRef<const char *> getRegMaskNames() const = 0;
 

llvm/lib/CodeGen/RegUsageInfoCollector.cpp

bool RegUsageInfoCollector::runOnMachineFunction(MachineFunction &MF) {
 
   BitVector SavedRegs;
   computeCalleeSavedRegs(SavedRegs, MF);
 
   const BitVector &UsedPhysRegsMask = MRI->getUsedPhysRegsMask();
   auto SetRegAsDefined = [&RegMask] (unsigned Reg) {
     RegMask[Reg / 32] &= ~(1u << Reg % 32);
   };
+
+  // Some targets can clobber registers "inside" a call, typically in
+  // linker-generated code.
+  for (const MCPhysReg *Reg = TRI->getIntraCallClobberedRegs(&MF); Reg && *Reg;
+       ++Reg)
+    for (MCRegAliasIterator AI(*Reg, TRI, true); AI.isValid(); ++AI)
+      SetRegAsDefined(*AI);
+
   // Scan all the physical registers. When a register is defined in the current
   // function set it and all the aliasing registers as defined in the regmask.
   // FIXME: Rewrite to use regunits.
   for (unsigned PReg = 1, PRegE = TRI->getNumRegs(); PReg < PRegE; ++PReg) {
     // Don't count registers that are saved and restored.
     if (SavedRegs.test(PReg))
       continue;
     // If a register is defined by an instruction mark it as defined together

llvm/lib/Target/ARM/ARMBaseRegisterInfo.h

public:
   /// (i.e. it is a calling convention that uses the same register for the first
   /// i32 argument and an i32 return value)
   ///
   /// Should return NULL in the case that the calling convention does not have
   /// this property
   const uint32_t *getThisReturnPreservedMask(const MachineFunction &MF,
                                              CallingConv::ID) const;
 
+  const MCPhysReg *
+  getIntraCallClobberedRegs(const MachineFunction *MF) const override;
+
   BitVector getReservedRegs(const MachineFunction &MF) const override;
   bool isAsmClobberable(const MachineFunction &MF,
                        unsigned PhysReg) const override;
 
   const TargetRegisterClass *
   getPointerRegClass(const MachineFunction &MF,
                      unsigned Kind = 0) const override;
   const TargetRegisterClass *

llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp

ARMBaseRegisterInfo::getThisReturnPreservedMask(const MachineFunction &MF,
   // should return NULL
   if (CC == CallingConv::GHC)
     // This is academic because all GHC calls are (supposed to be) tail calls
     return nullptr;
   return STI.isTargetDarwin() ? CSR_iOS_ThisReturn_RegMask
                               : CSR_AAPCS_ThisReturn_RegMask;
 }
 
+const MCPhysReg *ARMBaseRegisterInfo::getIntraCallClobberedRegs(
+    const MachineFunction *MF) const {
+  static MCPhysReg IntraCallClobberedRegs[] = {ARM::R12, 0};
+  return IntraCallClobberedRegs;
+}
+
 BitVector ARMBaseRegisterInfo::
 getReservedRegs(const MachineFunction &MF) const {
   const ARMSubtarget &STI = MF.getSubtarget<ARMSubtarget>();
   const ARMFrameLowering *TFI = getFrameLowering(MF);
 
   // FIXME: avoid re-calculating this every time.
   BitVector Reserved(getNumRegs());
   markSuperRegs(Reserved, ARM::SP);

llvm/lib/Target/ARM/ARMFrameLowering.cpp

if (BigFrameOffsets && !ExtraCSSpill) {
       }
       if (NumExtras == 0) {
         for (unsigned Reg : Extras) {
           SavedRegs.set(Reg);
           if (!MRI.isPhysRegUsed(Reg))
             ExtraCSSpill = true;
         }
       }
-      if (!ExtraCSSpill) {
+      if (!ExtraCSSpill && RS) {
         // Reserve a slot closest to SP or frame pointer.
-        assert(RS && "Register scavenging not provided");
         LLVM_DEBUG(dbgs() << "Reserving emergency spill slot\n");
         const TargetRegisterClass &RC = ARM::GPRRegClass;
         unsigned Size = TRI->getSpillSize(RC);
         unsigned Align = TRI->getSpillAlignment(RC);
         RS->addScavengingFrameIndex(MFI.CreateStackObject(Size, Align, false));
       }
     }
   }

llvm/test/CodeGen/ARM/ipra.ll

+; RUN: llc -mtriple armv7a--none-eabi < %s              | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLED
+; RUN: llc -mtriple armv7a--none-eabi < %s -enable-ipra | FileCheck %s --check-prefix=CHECK --check-prefix=ENABLED
+
+define void @leaf()  {
+entry:
+  tail call void asm sideeffect "", ""()
+  ret void
+}
+
+define void @leaf_r3() {
+entry:
+  tail call void asm sideeffect "", "~{r3}"()
+  ret void
+}
+
+define void @leaf_r4() {
+entry:
+  tail call void asm sideeffect "", "~{r4}"()
+  ret void
+}
+
+define void @leaf_s0() {
+entry:
+  tail call void asm sideeffect "", "~{s0}"()
+  ret void
+}
+
+define void @leaf_d0() {
+entry:
+  tail call void asm sideeffect "", "~{d0}"()
+  ret void
+}
+
+; r3 is normally caller-saved, but with IPRA we can see that it isn't used in
+; the callee, so can leave a live value in it.
+define void @test_r3_presrved() {
+; CHECK-LABEL: test_r3_presrved:
+entry:
+; CHECK: ASM1: r3
+; DISABLED: mov [[TEMP:r[0-9]+]], r3
+; ENABLED-NOT: r3
+; CHECK: bl      leaf
+; DISABLED: mov r3, [[TEMP]]
+; ENABLED-NOT: r3
+; CHECK: ASM2: r3
+  %a = tail call i32 asm sideeffect "// ASM1: $0", "={r3},0"(i32 undef)
+  tail call void @leaf()
+  %b = tail call i32 asm sideeffect "// ASM2: $0", "={r3},0"(i32 %a)
+  ret void
+}
+
+; Same as above, but r3 is clobbered in the callee, so it is clobbered by the
+; call as normal.
+define void @test_r3_clobbered() {
+; CHECK-LABEL: test_r3_clobbered:
+entry:
+; CHECK: ASM1: r3
+; CHECK: mov [[TEMP:r[0-9]+]], r3
+; CHECK: bl      leaf
+; CHECK: mov r3, [[TEMP]]
+; CHECK: ASM2: r3
+  %a = tail call i32 asm sideeffect "// ASM1: $0", "={r3},0"(i32 undef)
+  tail call void @leaf_r3()
+  %b = tail call i32 asm sideeffect "// ASM2: $0", "={r3},0"(i32 %a)
+  ret void
+}
+
+; r4 is a callee-saved register, so IPRA has no effect.
+define void @test_r4_preserved() {
+; CHECK-LABEL: test_r4_preserved:
+entry:
+; CHECK: ASM1: r4
+; CHECK-NOT: r4
+; CHECK: bl      leaf
+; CHECK-NOT: r4
+; CHECK: ASM2: r4
+  %a = tail call i32 asm sideeffect "// ASM1: $0", "={r4},0"(i32 undef)
+  tail call void @leaf()
+  %b = tail call i32 asm sideeffect "// ASM2: $0", "={r4},0"(i32 %a)
+  ret void
+}
+define void @test_r4_clobbered() {
+; CHECK-LABEL: test_r4_clobbered:
+entry:
+; CHECK: ASM1: r4
+; CHECK-NOT: r4
+; CHECK: bl      leaf_r4
+; CHECK-NOT: r4
+; CHECK: ASM2: r4
+  %a = tail call i32 asm sideeffect "// ASM1: $0", "={r4},0"(i32 undef)
+  tail call void @leaf_r4()
+  %b = tail call i32 asm sideeffect "// ASM2: $0", "={r4},0"(i32 %a)
+  ret void
+}
+
+; r12 is the intra-call scratch register, so we have to assume it is clobbered
+; even if we can see that the callee does not touch it.
+define void @test_r12() {
+; CHECK-LABEL: test_r12:
+entry:
+; CHECK: ASM1: r12
+; CHECK: mov [[TEMP:r[0-9]+]], r12
+; CHECK: bl      leaf
+; CHECK: mov r12, [[TEMP]]
+; CHECK: ASM2: r12
+  %a = tail call i32 asm sideeffect "// ASM1: $0", "={r12},0"(i32 undef)
+  tail call void @leaf()
+  %b = tail call i32 asm sideeffect "// ASM2: $0", "={r12},0"(i32 %a)
+  ret void
+}
+
+; s0 and d0 are caller-saved, IPRA allows us to keep them live in the caller if
+; the callee doesn't modify them.
+define void @test_s0_preserved() {
+; CHECK-LABEL: test_s0_preserved:
+entry:
+; CHECK: ASM1: s0
+; DISABLED: vmov.f32 [[TEMP:s[0-9]+]], s0
+; ENABLED-NOT: s0
+; CHECK: bl      leaf
+; DISABLED: vmov.f32 s0, [[TEMP]]
+; ENABLED-NOT: s0
+; CHECK: ASM2: s0
+  %a = tail call float asm sideeffect "// ASM1: $0", "={s0},0"(float undef)
+  tail call void @leaf()
+  %b = tail call float asm sideeffect "// ASM2: $0", "={s0},0"(float %a)
+  ret void
+}
+
+define void @test_s0_clobbered() {
+; CHECK-LABEL: test_s0_clobbered:
+entry:
+; CHECK: ASM1: s0
+; CHECK: vmov.f32 [[TEMP:s[0-9]+]], s0
+; CHECK: bl      leaf_s0
+; CHECK: vmov.f32 s0, [[TEMP]]
+; CHECK: ASM2: s0
+  %a = tail call float asm sideeffect "// ASM1: $0", "={s0},0"(float undef)
+  tail call void @leaf_s0()
+  %b = tail call float asm sideeffect "// ASM2: $0", "={s0},0"(float %a)
+  ret void
+}
+
+define void @test_d0_preserved() {
+; CHECK-LABEL: test_d0_preserved:
+entry:
+; CHECK: ASM1: d0
+; DISABLED: vmov.f64 [[TEMP:d[0-9]+]], d0
+; ENABLED-NOT: d0
+; CHECK: bl      leaf
+; DISABLED: vmov.f64 d0, [[TEMP]]
+; ENABLED-NOT: d0
+; CHECK: ASM2: d0
+  %a = tail call double asm sideeffect "// ASM1: $0", "={d0},0"(double undef)
+  tail call void @leaf()
+  %b = tail call double asm sideeffect "// ASM2: $0", "={d0},0"(double %a)
+  ret void
+}
+
+define void @test_d0_clobbered() {
+; CHECK-LABEL: test_d0_clobbered:
+entry:
+; CHECK: ASM1: d0
+; CHECK: vmov.f64 [[TEMP:d[0-9]+]], d0
+; CHECK: bl      leaf_d0
+; CHECK: vmov.f64 d0, [[TEMP]]
+; CHECK: ASM2: d0
+  %a = tail call double asm sideeffect "// ASM1: $0", "={d0},0"(double undef)
+  tail call void @leaf_d0()
+  %b = tail call double asm sideeffect "// ASM2: $0", "={d0},0"(double %a)
+  ret void
+}
+
+; s0 and d0 overlap, so clobbering one in the callee prevents the other from
+; being kept live across the call.
+define void @test_s0_clobber_d0() {
+; CHECK-LABEL: test_s0_clobber_d0:
+entry:
+; CHECK: ASM1: s0
+; CHECK: vmov.f32 [[TEMP:s[0-9]+]], s0
+; CHECK: bl      leaf_d0
+; CHECK: vmov.f32 s0, [[TEMP]]
+; CHECK: ASM2: s0
+  %a = tail call float asm sideeffect "// ASM1: $0", "={s0},0"(float undef)
+  tail call void @leaf_d0()
+  %b = tail call float asm sideeffect "// ASM2: $0", "={s0},0"(float %a)
+  ret void
+}
+
+define void @test_d0_clobber_s0() {
+; CHECK-LABEL: test_d0_clobber_s0:
+entry:
+; CHECK: ASM1: d0
+; CHECK: vmov.f64 [[TEMP:d[0-9]+]], d0
+; CHECK: bl      leaf_s0
+; CHECK: vmov.f64 d0, [[TEMP]]
+; CHECK: ASM2: d0
+  %a = tail call double asm sideeffect "// ASM1: $0", "={d0},0"(double undef)
+  tail call void @leaf_s0()
+  %b = tail call double asm sideeffect "// ASM2: $0", "={d0},0"(double %a)
+  ret void
+}