From 6a8700301ca6f8f2f5f787c8d1f5206a7dfceed6 Mon Sep 17 00:00:00 2001 From: Daniel Dunbar Date: Fri, 3 Sep 2010 15:26:42 +0000 Subject: [PATCH] Revert "For ARM stack frames that utilize variable sized objects and have either", it is breaking oggenc with Clang for ARMv6. This reverts commit 8d6e29cfda270be483abf638850311670829ee65. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@112962 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/ARMBaseRegisterInfo.cpp | 99 +++++-------------- lib/Target/ARM/ARMBaseRegisterInfo.h | 6 -- lib/Target/ARM/Thumb1RegisterInfo.cpp | 16 +-- test/CodeGen/Thumb/dyn-stackalloc.ll | 2 +- test/CodeGen/Thumb2/2010-04-15-DynAllocBug.ll | 11 ++- 5 files changed, 36 insertions(+), 98 deletions(-) diff --git a/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/lib/Target/ARM/ARMBaseRegisterInfo.cpp index df6d00d400c..99c2e997e7d 100644 --- a/lib/Target/ARM/ARMBaseRegisterInfo.cpp +++ b/lib/Target/ARM/ARMBaseRegisterInfo.cpp @@ -50,10 +50,6 @@ EnableLocalStackAlloc("enable-local-stack-alloc", cl::init(true), cl::Hidden, using namespace llvm; -static cl::opt -EnableBasePointer("arm-use-base-pointer", cl::Hidden, cl::init(true), - cl::desc("Enable use of a base pointer for complex stack frames")); - unsigned ARMBaseRegisterInfo::getRegisterNumbering(unsigned RegEnum, bool *isSPVFP) { if (isSPVFP) @@ -150,8 +146,7 @@ ARMBaseRegisterInfo::ARMBaseRegisterInfo(const ARMBaseInstrInfo &tii, const ARMSubtarget &sti) : ARMGenRegisterInfo(ARM::ADJCALLSTACKDOWN, ARM::ADJCALLSTACKUP), TII(tii), STI(sti), - FramePtr((STI.isTargetDarwin() || STI.isThumb()) ? ARM::R7 : ARM::R11), - BasePtr(ARM::R6) { + FramePtr((STI.isTargetDarwin() || STI.isThumb()) ? ARM::R7 : ARM::R11) { } const unsigned* @@ -187,8 +182,6 @@ getReservedRegs(const MachineFunction &MF) const { Reserved.set(ARM::FPSCR); if (hasFP(MF)) Reserved.set(FramePtr); - if (hasBasePointer(MF)) - Reserved.set(BasePtr); // Some targets reserve R9. if (STI.isR9Reserved()) Reserved.set(ARM::R9); @@ -202,10 +195,6 @@ bool ARMBaseRegisterInfo::isReservedReg(const MachineFunction &MF, case ARM::SP: case ARM::PC: return true; - case ARM::R6: - if (hasBasePointer(MF)) - return true; - break; case ARM::R7: case ARM::R11: if (FramePtr == Reg && hasFP(MF)) @@ -636,49 +625,35 @@ bool ARMBaseRegisterInfo::hasFP(const MachineFunction &MF) const { MFI->isFrameAddressTaken()); } -bool ARMBaseRegisterInfo::hasBasePointer(const MachineFunction &MF) const { - const MachineFrameInfo *MFI = MF.getFrameInfo(); - const ARMFunctionInfo *AFI = MF.getInfo(); - - if (!EnableBasePointer) - return false; - - if (needsStackRealignment(MF) && MFI->hasVarSizedObjects()) - return true; - - // Thumb has trouble with negative offsets from the FP. Thumb2 has a limited - // negative range for ldr/str (255), and thumb1 is positive offsets only. - // It's going to be better to use the SP or Base Pointer instead. When there - // are variable sized objects, we can't reference off of the SP, so we - // reserve a Base Pointer. - if (AFI->isThumbFunction() && MFI->hasVarSizedObjects()) { - // Conservatively estimate whether the negative offset from the frame - // pointer will be sufficient to reach. If a function has a smallish - // frame, it's less likely to have lots of spills and callee saved - // space, so it's all more likely to be within range of the frame pointer. - // If it's wrong, the scavenger will still enable access to work, it just - // won't be optimal. - if (AFI->isThumb2Function() && MFI->getLocalFrameSize() < 128) - return false; - return true; - } - - return false; -} - bool ARMBaseRegisterInfo::canRealignStack(const MachineFunction &MF) const { + const MachineFrameInfo *MFI = MF.getFrameInfo(); const ARMFunctionInfo *AFI = MF.getInfo(); - return (RealignStack && !AFI->isThumb1OnlyFunction()); + return (RealignStack && + !AFI->isThumb1OnlyFunction() && + !MFI->hasVarSizedObjects()); } bool ARMBaseRegisterInfo:: needsStackRealignment(const MachineFunction &MF) const { const MachineFrameInfo *MFI = MF.getFrameInfo(); const Function *F = MF.getFunction(); + const ARMFunctionInfo *AFI = MF.getInfo(); unsigned StackAlign = MF.getTarget().getFrameInfo()->getStackAlignment(); bool requiresRealignment = ((MFI->getMaxAlignment() > StackAlign) || F->hasFnAttr(Attribute::StackAlignment)); + // FIXME: Currently we don't support stack realignment for functions with + // variable-sized allocas. + // FIXME: It's more complicated than this... + if (0 && requiresRealignment && MFI->hasVarSizedObjects()) + report_fatal_error( + "Stack realignment in presense of dynamic allocas is not supported"); + + // FIXME: This probably isn't the right place for this. + if (0 && requiresRealignment && AFI->isThumb1OnlyFunction()) + report_fatal_error( + "Stack realignment in thumb1 functions is not supported"); + return requiresRealignment && canRealignStack(MF); } @@ -801,10 +776,6 @@ ARMBaseRegisterInfo::processFunctionBeforeCalleeSavedScan(MachineFunction &MF, if (AFI->isThumb1OnlyFunction() && AFI->getVarArgsRegSaveSize() > 0) MF.getRegInfo().setPhysRegUsed(ARM::LR); - // Spill the BasePtr if it's used. - if (hasBasePointer(MF)) - MF.getRegInfo().setPhysRegUsed(BasePtr); - // Don't spill FP if the frame can be eliminated. This is determined // by scanning the callee-save registers to see if any is used. const unsigned *CSRegs = getCalleeSavedRegs(); @@ -1051,14 +1022,13 @@ ARMBaseRegisterInfo::ResolveFrameIndexReference(const MachineFunction &MF, return Offset - AFI->getDPRCalleeSavedAreaOffset(); // When dynamically realigning the stack, use the frame pointer for - // parameters, and the stack/base pointer for locals. + // parameters, and the stack pointer for locals. if (needsStackRealignment(MF)) { assert (hasFP(MF) && "dynamic stack realignment without a FP!"); if (isFixed) { FrameReg = getFrameRegister(MF); Offset = FPOffset; - } else if (MFI->hasVarSizedObjects()) - FrameReg = BasePtr; + } return Offset; } @@ -1066,13 +1036,9 @@ ARMBaseRegisterInfo::ResolveFrameIndexReference(const MachineFunction &MF, if (hasFP(MF) && AFI->hasStackFrame()) { // Use frame pointer to reference fixed objects. Use it for locals if // there are VLAs (and thus the SP isn't reliable as a base). - if (isFixed || (MFI->hasVarSizedObjects() && !hasBasePointer(MF))) { + if (isFixed || MFI->hasVarSizedObjects()) { FrameReg = getFrameRegister(MF); Offset = FPOffset; - } else if (MFI->hasVarSizedObjects()) { - assert(hasBasePointer(MF) && "missing base pointer!"); - // Use the base register since we have it. - FrameReg = BasePtr; } else if (AFI->isThumb2Function()) { // In Thumb2 mode, the negative offset is very limited. Try to avoid // out of range references. @@ -1086,9 +1052,6 @@ ARMBaseRegisterInfo::ResolveFrameIndexReference(const MachineFunction &MF, Offset = FPOffset; } } - // Use the base pointer if we have one. - if (hasBasePointer(MF)) - FrameReg = BasePtr; return Offset; } @@ -1126,8 +1089,7 @@ unsigned ARMBaseRegisterInfo::getRegisterPairEven(unsigned Reg, case ARM::R5: return ARM::R4; case ARM::R7: - return (isReservedReg(MF, ARM::R7) || isReservedReg(MF, ARM::R6)) - ? 0 : ARM::R6; + return isReservedReg(MF, ARM::R7) ? 0 : ARM::R6; case ARM::R9: return isReservedReg(MF, ARM::R9) ? 0 :ARM::R8; case ARM::R11: @@ -1216,8 +1178,7 @@ unsigned ARMBaseRegisterInfo::getRegisterPairOdd(unsigned Reg, case ARM::R4: return ARM::R5; case ARM::R6: - return (isReservedReg(MF, ARM::R7) || isReservedReg(MF, ARM::R6)) - ? 0 : ARM::R7; + return isReservedReg(MF, ARM::R7) ? 0 : ARM::R7; case ARM::R8: return isReservedReg(MF, ARM::R9) ? 0 :ARM::R9; case ARM::R10: @@ -1917,20 +1878,6 @@ emitPrologue(MachineFunction &MF) const { AFI->setShouldRestoreSPFromFP(true); } - // If we need a base pointer, set it up here. It's whatever the value - // of the stack pointer is at this point. Any variable size objects - // will be allocated after this, so we can still use the base pointer - // to reference locals. - if (hasBasePointer(MF)) { - if (isARM) - BuildMI(MBB, MBBI, dl, TII.get(ARM::MOVr), BasePtr) - .addReg(ARM::SP) - .addImm((unsigned)ARMCC::AL).addReg(0).addReg(0); - else - BuildMI(MBB, MBBI, dl, TII.get(ARM::tMOVgpr2gpr), BasePtr) - .addReg(ARM::SP); - } - // If the frame has variable sized objects then the epilogue must restore // the sp from fp. if (!AFI->shouldRestoreSPFromFP() && MFI->hasVarSizedObjects()) diff --git a/lib/Target/ARM/ARMBaseRegisterInfo.h b/lib/Target/ARM/ARMBaseRegisterInfo.h index fa2eb6c1049..c814f998d5b 100644 --- a/lib/Target/ARM/ARMBaseRegisterInfo.h +++ b/lib/Target/ARM/ARMBaseRegisterInfo.h @@ -52,11 +52,6 @@ protected: /// FramePtr - ARM physical register used as frame ptr. unsigned FramePtr; - /// BasePtr - ARM physical register used as a base ptr in complex stack - /// frames. I.e., when we need a 3rd base, not just SP and FP, due to - /// variable size stack objects. - unsigned BasePtr; - // Can be only subclassed. explicit ARMBaseRegisterInfo(const ARMBaseInstrInfo &tii, const ARMSubtarget &STI); @@ -107,7 +102,6 @@ public: MachineFunction &MF) const; bool hasFP(const MachineFunction &MF) const; - bool hasBasePointer(const MachineFunction &MF) const; bool canRealignStack(const MachineFunction &MF) const; bool needsStackRealignment(const MachineFunction &MF) const; diff --git a/lib/Target/ARM/Thumb1RegisterInfo.cpp b/lib/Target/ARM/Thumb1RegisterInfo.cpp index a21a3da10bd..c562c371469 100644 --- a/lib/Target/ARM/Thumb1RegisterInfo.cpp +++ b/lib/Target/ARM/Thumb1RegisterInfo.cpp @@ -604,12 +604,9 @@ Thumb1RegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II, else if (MF.getFrameInfo()->hasVarSizedObjects()) { assert(SPAdj == 0 && hasFP(MF) && "Unexpected"); // There are alloca()'s in this function, must reference off the frame - // pointer or base pointer instead. - if (!hasBasePointer(MF)) { - FrameReg = getFrameRegister(MF); - Offset -= AFI->getFramePtrSpillOffset(); - } else - FrameReg = BasePtr; + // pointer instead. + FrameReg = getFrameRegister(MF); + Offset -= AFI->getFramePtrSpillOffset(); } // Special handling of dbg_value instructions. @@ -790,13 +787,6 @@ void Thumb1RegisterInfo::emitPrologue(MachineFunction &MF) const { AFI->setGPRCalleeSavedArea1Size(GPRCS1Size); AFI->setGPRCalleeSavedArea2Size(GPRCS2Size); AFI->setDPRCalleeSavedAreaSize(DPRCSSize); - - // If we need a base pointer, set it up here. It's whatever the value - // of the stack pointer is at this point. Any variable size objects - // will be allocated after this, so we can still use the base pointer - // to reference locals. - if (hasBasePointer(MF)) - BuildMI(MBB, MBBI, dl, TII.get(ARM::tMOVgpr2gpr), BasePtr).addReg(ARM::SP); } static bool isCalleeSavedRegister(unsigned Reg, const unsigned *CSRegs) { diff --git a/test/CodeGen/Thumb/dyn-stackalloc.ll b/test/CodeGen/Thumb/dyn-stackalloc.ll index 5c8ad974bc0..acfdc917ddf 100644 --- a/test/CodeGen/Thumb/dyn-stackalloc.ll +++ b/test/CodeGen/Thumb/dyn-stackalloc.ll @@ -1,7 +1,7 @@ ; RUN: llc < %s -march=thumb | not grep {ldr sp} ; RUN: llc < %s -mtriple=thumb-apple-darwin | \ ; RUN: not grep {sub.*r7} -; RUN: llc < %s -march=thumb | grep {mov.*r6, sp} +; RUN: llc < %s -march=thumb | grep 4294967280 %struct.state = type { i32, %struct.info*, float**, i32, i32, i32, i32, i32, i32, i32, i32, i32, i64, i64, i64, i64, i64, i64, i8* } %struct.info = type { i32, i32, i32, i32, i32, i32, i32, i8* } diff --git a/test/CodeGen/Thumb2/2010-04-15-DynAllocBug.ll b/test/CodeGen/Thumb2/2010-04-15-DynAllocBug.ll index 2246de35e03..e0946c7ea36 100644 --- a/test/CodeGen/Thumb2/2010-04-15-DynAllocBug.ll +++ b/test/CodeGen/Thumb2/2010-04-15-DynAllocBug.ll @@ -7,12 +7,19 @@ define void @t() nounwind ssp { entry: ; CHECK: t: - %size = mul i32 8, 2 +; CHECK: push {r4, r7} +; CHECK: mov r0, sp +; CHECK: add r7, sp, #4 +; CHECK: bic r0, r0, #7 ; CHECK: subs r0, #16 ; CHECK: mov sp, r0 - %vla_a = alloca i8, i32 %size, align 8 +; CHECK: mov r0, sp +; CHECK: bic r0, r0, #7 ; CHECK: subs r0, #16 ; CHECK: mov sp, r0 + + %size = mul i32 8, 2 + %vla_a = alloca i8, i32 %size, align 8 %vla_b = alloca i8, i32 %size, align 8 unreachable } -- 2.34.1