[llvm] r232825 - [ARM] Fix handling of thumb1 out-of-range frame offsets
John Brawn
john.brawn at arm.com
Fri Mar 20 10:20:08 PDT 2015
Author: john.brawn
Date: Fri Mar 20 12:20:07 2015
New Revision: 232825
URL: http://llvm.org/viewvc/llvm-project?rev=232825&view=rev
Log:
[ARM] Fix handling of thumb1 out-of-range frame offsets
LocalStackSlotPass assumes that isFrameOffsetLegal doesn't change its
answer when the base register changes. Unfortunately this isn't true
in thumb1, where SP-based loads allow a larger offset than
non-SP-based loads, and this causes the base register reuse code to
generate instructions that are unencodable, causing an assertion
failure.
Solve this by adding a BaseReg parameter to isFrameOffsetLegal, which
ARMBaseRegisterInfo can then make use of to give the correct answer.
Differential Revision: http://reviews.llvm.org/D8419
Modified:
llvm/trunk/include/llvm/Target/TargetRegisterInfo.h
llvm/trunk/lib/CodeGen/LocalStackSlotAllocation.cpp
llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h
llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp
llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h
llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp
llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.h
llvm/trunk/test/CodeGen/Thumb/stack-access.ll
Modified: llvm/trunk/include/llvm/Target/TargetRegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetRegisterInfo.h?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Target/TargetRegisterInfo.h (original)
+++ llvm/trunk/include/llvm/Target/TargetRegisterInfo.h Fri Mar 20 12:20:07 2015
@@ -799,9 +799,9 @@ public:
llvm_unreachable("resolveFrameIndex does not exist on this target");
}
- /// isFrameOffsetLegal - Determine whether a given offset immediate is
- /// encodable to resolve a frame index.
- virtual bool isFrameOffsetLegal(const MachineInstr *MI,
+ /// isFrameOffsetLegal - Determine whether a given base register plus offset
+ /// immediate is encodable to resolve a frame index.
+ virtual bool isFrameOffsetLegal(const MachineInstr *MI, unsigned BaseReg,
int64_t Offset) const {
llvm_unreachable("isFrameOffsetLegal does not exist on this target");
}
Modified: llvm/trunk/lib/CodeGen/LocalStackSlotAllocation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LocalStackSlotAllocation.cpp?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/LocalStackSlotAllocation.cpp (original)
+++ llvm/trunk/lib/CodeGen/LocalStackSlotAllocation.cpp Fri Mar 20 12:20:07 2015
@@ -252,7 +252,8 @@ void LocalStackSlotPass::calculateFrameO
}
static inline bool
-lookupCandidateBaseReg(int64_t BaseOffset,
+lookupCandidateBaseReg(unsigned BaseReg,
+ int64_t BaseOffset,
int64_t FrameSizeAdjust,
int64_t LocalFrameOffset,
const MachineInstr *MI,
@@ -260,7 +261,7 @@ lookupCandidateBaseReg(int64_t BaseOffse
// Check if the relative offset from the where the base register references
// to the target address is in range for the instruction.
int64_t Offset = FrameSizeAdjust + LocalFrameOffset - BaseOffset;
- return TRI->isFrameOffsetLegal(MI, Offset);
+ return TRI->isFrameOffsetLegal(MI, BaseReg, Offset);
}
bool LocalStackSlotPass::insertFrameReferenceRegisters(MachineFunction &Fn) {
@@ -362,8 +363,9 @@ bool LocalStackSlotPass::insertFrameRefe
// instruction itself will be taken into account by the target,
// so we don't have to adjust for it here when reusing a base
// register.
- if (UsedBaseReg && lookupCandidateBaseReg(BaseOffset, FrameSizeAdjust,
- LocalOffset, MI, TRI)) {
+ if (UsedBaseReg && lookupCandidateBaseReg(BaseReg, BaseOffset,
+ FrameSizeAdjust, LocalOffset, MI,
+ TRI)) {
DEBUG(dbgs() << " Reusing base register " << BaseReg << "\n");
// We found a register to reuse.
Offset = FrameSizeAdjust + LocalOffset - BaseOffset;
@@ -382,7 +384,7 @@ bool LocalStackSlotPass::insertFrameRefe
// then don't bother creating it.
if (ref + 1 >= e ||
!lookupCandidateBaseReg(
- BaseOffset, FrameSizeAdjust,
+ BaseReg, BaseOffset, FrameSizeAdjust,
FrameReferenceInsns[ref + 1].getLocalOffset(),
FrameReferenceInsns[ref + 1].getMachineInstr(), TRI)) {
BaseOffset = PrevBaseOffset;
Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp Fri Mar 20 12:20:07 2015
@@ -271,7 +271,7 @@ bool AArch64RegisterInfo::needsFrameBase
// The FP is only available if there is no dynamic realignment. We
// don't know for sure yet whether we'll need that, so we guess based
// on whether there are any local variables that would trigger it.
- if (TFI->hasFP(MF) && isFrameOffsetLegal(MI, FPOffset))
+ if (TFI->hasFP(MF) && isFrameOffsetLegal(MI, AArch64::FP, FPOffset))
return false;
// If we can reference via the stack pointer or base pointer, try that.
@@ -279,7 +279,7 @@ bool AArch64RegisterInfo::needsFrameBase
// to only disallow SP relative references in the live range of
// the VLA(s). In practice, it's unclear how much difference that
// would make, but it may be worth doing.
- if (isFrameOffsetLegal(MI, Offset))
+ if (isFrameOffsetLegal(MI, AArch64::SP, Offset))
return false;
// The offset likely isn't legal; we want to allocate a virtual base register.
@@ -287,6 +287,7 @@ bool AArch64RegisterInfo::needsFrameBase
}
bool AArch64RegisterInfo::isFrameOffsetLegal(const MachineInstr *MI,
+ unsigned BaseReg,
int64_t Offset) const {
assert(Offset <= INT_MAX && "Offset too big to fit in int.");
assert(MI && "Unable to get the legal offset for nil instruction.");
Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.h Fri Mar 20 12:20:07 2015
@@ -72,7 +72,7 @@ public:
bool requiresFrameIndexScavenging(const MachineFunction &MF) const override;
bool needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const override;
- bool isFrameOffsetLegal(const MachineInstr *MI,
+ bool isFrameOffsetLegal(const MachineInstr *MI, unsigned BaseReg,
int64_t Offset) const override;
void materializeFrameBaseRegister(MachineBasicBlock *MBB, unsigned BaseReg,
int FrameIdx,
Modified: llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp Fri Mar 20 12:20:07 2015
@@ -536,9 +536,8 @@ needsFrameBaseReg(MachineInstr *MI, int6
// on whether there are any local variables that would trigger it.
unsigned StackAlign = TFI->getStackAlignment();
if (TFI->hasFP(MF) &&
- (MI->getDesc().TSFlags & ARMII::AddrModeMask) != ARMII::AddrModeT1_s &&
!((MFI->getLocalFrameMaxAlign() > StackAlign) && canRealignStack(MF))) {
- if (isFrameOffsetLegal(MI, FPOffset))
+ if (isFrameOffsetLegal(MI, getFrameRegister(MF), FPOffset))
return false;
}
// If we can reference via the stack pointer, try that.
@@ -546,7 +545,7 @@ needsFrameBaseReg(MachineInstr *MI, int6
// to only disallow SP relative references in the live range of
// the VLA(s). In practice, it's unclear how much difference that
// would make, but it may be worth doing.
- if (!MFI->hasVarSizedObjects() && isFrameOffsetLegal(MI, Offset))
+ if (!MFI->hasVarSizedObjects() && isFrameOffsetLegal(MI, ARM::SP, Offset))
return false;
// The offset likely isn't legal, we want to allocate a virtual base register.
@@ -609,7 +608,7 @@ void ARMBaseRegisterInfo::resolveFrameIn
(void)Done;
}
-bool ARMBaseRegisterInfo::isFrameOffsetLegal(const MachineInstr *MI,
+bool ARMBaseRegisterInfo::isFrameOffsetLegal(const MachineInstr *MI, unsigned BaseReg,
int64_t Offset) const {
const MCInstrDesc &Desc = MI->getDesc();
unsigned AddrMode = (Desc.TSFlags & ARMII::AddrModeMask);
@@ -653,7 +652,7 @@ bool ARMBaseRegisterInfo::isFrameOffsetL
NumBits = 8;
break;
case ARMII::AddrModeT1_s:
- NumBits = 8;
+ NumBits = (BaseReg == ARM::SP ? 8 : 5);
Scale = 4;
isSigned = false;
break;
Modified: llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h (original)
+++ llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.h Fri Mar 20 12:20:07 2015
@@ -143,7 +143,7 @@ public:
int64_t Offset) const override;
void resolveFrameIndex(MachineInstr &MI, unsigned BaseReg,
int64_t Offset) const override;
- bool isFrameOffsetLegal(const MachineInstr *MI,
+ bool isFrameOffsetLegal(const MachineInstr *MI, unsigned BaseReg,
int64_t Offset) const override;
bool cannotEliminateFrame(const MachineFunction &MF) const;
Modified: llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp Fri Mar 20 12:20:07 2015
@@ -993,7 +993,7 @@ needsFrameBaseReg(MachineInstr *MI, int6
// The frame pointer will point to the end of the stack, so estimate the
// offset as the difference between the object offset and the FP location.
- return !isFrameOffsetLegal(MI, Offset);
+ return !isFrameOffsetLegal(MI, getBaseRegister(MF), Offset);
}
/// Insert defining instruction(s) for BaseReg to
@@ -1045,6 +1045,7 @@ void PPCRegisterInfo::resolveFrameIndex(
}
bool PPCRegisterInfo::isFrameOffsetLegal(const MachineInstr *MI,
+ unsigned BaseReg,
int64_t Offset) const {
unsigned FIOperandNum = 0;
while (!MI->getOperand(FIOperandNum).isFI()) {
Modified: llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.h?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.h (original)
+++ llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.h Fri Mar 20 12:20:07 2015
@@ -94,7 +94,7 @@ public:
int64_t Offset) const override;
void resolveFrameIndex(MachineInstr &MI, unsigned BaseReg,
int64_t Offset) const override;
- bool isFrameOffsetLegal(const MachineInstr *MI,
+ bool isFrameOffsetLegal(const MachineInstr *MI, unsigned BaseReg,
int64_t Offset) const override;
// Debug information queries.
Modified: llvm/trunk/test/CodeGen/Thumb/stack-access.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Thumb/stack-access.ll?rev=232825&r1=232824&r2=232825&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/Thumb/stack-access.ll (original)
+++ llvm/trunk/test/CodeGen/Thumb/stack-access.ll Fri Mar 20 12:20:07 2015
@@ -88,3 +88,40 @@ define void @test7() {
ret void
}
+
+; Check that loads/stores with out-of-range offsets are handled correctly
+define void @test8() {
+ %arr3 = alloca [224 x i32], align 4
+ %arr2 = alloca [224 x i32], align 4
+ %arr1 = alloca [224 x i32], align 4
+
+; CHECK: movs [[REG:r[0-9]+]], #1
+; CHECK: str [[REG]], [sp]
+ %arr1idx1 = getelementptr inbounds [224 x i32], [224 x i32]* %arr1, i32 0, i32 0
+ store i32 1, i32* %arr1idx1, align 4
+
+; Offset in range for sp-based store, but not for non-sp-based store
+; CHECK: str [[REG]], [sp, #128]
+ %arr1idx2 = getelementptr inbounds [224 x i32], [224 x i32]* %arr1, i32 0, i32 32
+ store i32 1, i32* %arr1idx2, align 4
+
+; CHECK: str [[REG]], [sp, #896]
+ %arr2idx1 = getelementptr inbounds [224 x i32], [224 x i32]* %arr2, i32 0, i32 0
+ store i32 1, i32* %arr2idx1, align 4
+
+; %arr2 is in range, but this element of it is not
+; CHECK: str [[REG]], [{{r[0-9]+}}]
+ %arr2idx2 = getelementptr inbounds [224 x i32], [224 x i32]* %arr2, i32 0, i32 32
+ store i32 1, i32* %arr2idx2, align 4
+
+; %arr3 is not in range
+; CHECK: str [[REG]], [{{r[0-9]+}}]
+ %arr3idx1 = getelementptr inbounds [224 x i32], [224 x i32]* %arr3, i32 0, i32 0
+ store i32 1, i32* %arr3idx1, align 4
+
+; CHECK: str [[REG]], [{{r[0-9]+}}]
+ %arr3idx2 = getelementptr inbounds [224 x i32], [224 x i32]* %arr3, i32 0, i32 32
+ store i32 1, i32* %arr3idx2, align 4
+
+ ret void
+}
More information about the llvm-commits
mailing list