maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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

llvm-svn: 232825
This commit is contained in:
John Brawn 2015-03-20 17:20:07 +00:00
parent c403a1ce32
commit 1f26a47630
9 changed files with 59 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
llvm/include/llvm/Target/TargetRegisterInfo.h
View File

@ -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");
}

12
llvm/lib/CodeGen/LocalStackSlotAllocation.cpp
View File

@ -252,7 +252,8 @@ void LocalStackSlotPass::calculateFrameObjectOffsets(MachineFunction &Fn) {
}
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 BaseOffset,
// 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::insertFrameReferenceRegisters(MachineFunction &Fn) {
// 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::insertFrameReferenceRegisters(MachineFunction &Fn) {
// 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;

5
llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
View File

@ -271,7 +271,7 @@ bool AArch64RegisterInfo::needsFrameBaseReg(MachineInstr *MI,
// 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::needsFrameBaseReg(MachineInstr *MI,
// 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::needsFrameBaseReg(MachineInstr *MI,
}
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.");

2
llvm/lib/Target/AArch64/AArch64RegisterInfo.h
View File

@ -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,

9
llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
View File

@ -536,9 +536,8 @@ needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const {
// 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, int64_t Offset) const {
// 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::resolveFrameIndex(MachineInstr &MI, unsigned BaseReg,
(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::isFrameOffsetLegal(const MachineInstr *MI,
NumBits = 8;
break;
case ARMII::AddrModeT1_s:
NumBits = 8;
NumBits = (BaseReg == ARM::SP ? 8 : 5);
Scale = 4;
isSigned = false;
break;

2
llvm/lib/Target/ARM/ARMBaseRegisterInfo.h
View File

@ -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;

3
llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
View File

@ -993,7 +993,7 @@ needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const {
// 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(MachineInstr &MI, unsigned BaseReg,
}
bool PPCRegisterInfo::isFrameOffsetLegal(const MachineInstr *MI,
unsigned BaseReg,
int64_t Offset) const {
unsigned FIOperandNum = 0;
while (!MI->getOperand(FIOperandNum).isFI()) {

2
llvm/lib/Target/PowerPC/PPCRegisterInfo.h
View File

@ -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.

37
llvm/test/CodeGen/Thumb/stack-access.ll
View File

@ -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
}