[llvm] 965ed1e - [AArch64] Fix issues with large arrays on stack
Kiran Chandramohan via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 10 03:54:32 PST 2019
Author: Kiran Chandramohan
Date: 2019-12-10T11:44:41Z
New Revision: 965ed1e974e80e0b96ac3921e8a915e8e46baa5c
URL: https://github.com/llvm/llvm-project/commit/965ed1e974e80e0b96ac3921e8a915e8e46baa5c
DIFF: https://github.com/llvm/llvm-project/commit/965ed1e974e80e0b96ac3921e8a915e8e46baa5c.diff
LOG: [AArch64] Fix issues with large arrays on stack
Summary:
This patch fixes a few issues when large arrays are allocated on the
stack. Currently, clang has inconsistent behaviour, for debug builds
there is an assertion failure when the array size on stack is around 2GB
but there is no assertion when the stack is around 8GB. For release
builds there is no assertion, the compilation succeeds but generates
incorrect code. The incorrect code generated is due to using
int/unsigned int instead of their 64-bit counterparts. This patch,
1) Removes the assertion in frame legality check.
2) Converts int/unsigned int in some places to the 64-bit variants. This
helps in generating correct code and removes the inconsistent behaviour.
3) Adds a test which runs without optimisations.
Reviewers: sdesmalen, efriedma, fhahn, aemerson
Reviewed By: efriedma
Subscribers: eli.friedman, fpetrogalli, kristof.beyls, hiraditya,
llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D70496
Added:
llvm/test/CodeGen/AArch64/large-stack.ll
Modified:
llvm/include/llvm/CodeGen/MachineFrameInfo.h
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
llvm/lib/CodeGen/MachineFrameInfo.cpp
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
llvm/lib/Target/AArch64/AArch64FrameLowering.h
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 01fc50d14a7f..05b34d92651c 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -553,7 +553,7 @@ class MachineFrameInfo {
void setStackSize(uint64_t Size) { StackSize = Size; }
/// Estimate and return the size of the stack frame.
- unsigned estimateStackSize(const MachineFunction &MF) const;
+ uint64_t estimateStackSize(const MachineFunction &MF) const;
/// Return the correction for frame offsets.
int getOffsetAdjustment() const { return OffsetAdjustment; }
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 5b0ad083bfd9..223ed9548544 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -225,12 +225,12 @@ int IRTranslator::getOrCreateFrameIndex(const AllocaInst &AI) {
if (FrameIndices.find(&AI) != FrameIndices.end())
return FrameIndices[&AI];
- unsigned ElementSize = DL->getTypeAllocSize(AI.getAllocatedType());
- unsigned Size =
+ uint64_t ElementSize = DL->getTypeAllocSize(AI.getAllocatedType());
+ uint64_t Size =
ElementSize * cast<ConstantInt>(AI.getArraySize())->getZExtValue();
// Always allocate at least one byte.
- Size = std::max(Size, 1u);
+ Size = std::max<uint64_t>(Size, 1u);
unsigned Alignment = AI.getAlignment();
if (!Alignment)
diff --git a/llvm/lib/CodeGen/MachineFrameInfo.cpp b/llvm/lib/CodeGen/MachineFrameInfo.cpp
index 604f5145b1a0..22ab2c7a6d77 100644
--- a/llvm/lib/CodeGen/MachineFrameInfo.cpp
+++ b/llvm/lib/CodeGen/MachineFrameInfo.cpp
@@ -133,11 +133,11 @@ BitVector MachineFrameInfo::getPristineRegs(const MachineFunction &MF) const {
return BV;
}
-unsigned MachineFrameInfo::estimateStackSize(const MachineFunction &MF) const {
+uint64_t MachineFrameInfo::estimateStackSize(const MachineFunction &MF) const {
const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
unsigned MaxAlign = getMaxAlignment();
- int Offset = 0;
+ int64_t Offset = 0;
// This code is very, very similar to PEI::calculateFrameObjectOffsets().
// It really should be refactored to share code. Until then, changes
@@ -147,7 +147,7 @@ unsigned MachineFrameInfo::estimateStackSize(const MachineFunction &MF) const {
// Only estimate stack size of default stack.
if (getStackID(i) != TargetStackID::Default)
continue;
- int FixedOff = -getObjectOffset(i);
+ int64_t FixedOff = -getObjectOffset(i);
if (FixedOff > Offset) Offset = FixedOff;
}
for (unsigned i = 0, e = getObjectIndexEnd(); i != e; ++i) {
@@ -183,7 +183,7 @@ unsigned MachineFrameInfo::estimateStackSize(const MachineFunction &MF) const {
unsigned AlignMask = StackAlign - 1;
Offset = (Offset + AlignMask) & ~uint64_t(AlignMask);
- return (unsigned)Offset;
+ return (uint64_t)Offset;
}
void MachineFrameInfo::computeMaxCallFrameSize(const MachineFunction &MF) {
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 8f88198203d7..742f12fefcac 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -227,7 +227,7 @@ bool AArch64FrameLowering::canUseRedZone(const MachineFunction &MF) const {
const MachineFrameInfo &MFI = MF.getFrameInfo();
const AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
- unsigned NumBytes = AFI->getLocalStackSize();
+ uint64_t NumBytes = AFI->getLocalStackSize();
return !(MFI.hasCalls() || hasFP(MF) || NumBytes > 128 ||
getSVEStackSize(MF));
@@ -429,7 +429,7 @@ bool AArch64FrameLowering::canUseAsPrologue(
}
static bool windowsRequiresStackProbe(MachineFunction &MF,
- unsigned StackSizeInBytes) {
+ uint64_t StackSizeInBytes) {
const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
if (!Subtarget.isTargetWindows())
return false;
@@ -446,7 +446,7 @@ static bool windowsRequiresStackProbe(MachineFunction &MF,
}
bool AArch64FrameLowering::shouldCombineCSRLocalStackBump(
- MachineFunction &MF, unsigned StackBumpBytes) const {
+ MachineFunction &MF, uint64_t StackBumpBytes) const {
AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
const MachineFrameInfo &MFI = MF.getFrameInfo();
const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
@@ -725,7 +725,7 @@ static MachineBasicBlock::iterator convertCalleeSaveRestoreToSPPrePostIncDec(
// Fixup callee-save register save/restore instructions to take into account
// combined SP bump by adding the local stack size to the stack offsets.
static void fixupCalleeSaveRestoreStackOffset(MachineInstr &MI,
- unsigned LocalStackSize,
+ uint64_t LocalStackSize,
bool NeedsWinCFI,
bool *HasWinCFI) {
if (AArch64InstrInfo::isSEHInstruction(MI))
@@ -913,8 +913,8 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
// pointer from the funclet. We only save the callee saved registers in the
// funclet, which are really the callee saved registers of the parent
// function, including the funclet.
- int NumBytes = IsFunclet ? (int)getWinEHFuncletFrameSize(MF)
- : (int)MFI.getStackSize();
+ int64_t NumBytes = IsFunclet ? getWinEHFuncletFrameSize(MF)
+ : MFI.getStackSize();
if (!AFI->hasStackFrame() && !windowsRequiresStackProbe(MF, NumBytes)) {
assert(!HasFP && "unexpected function without stack frame but with FP");
assert(!SVEStackSize &&
@@ -1016,7 +1016,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
if (HasFP) {
// Only set up FP if we actually need to.
- int FPOffset = isTargetDarwin(MF) ? (AFI->getCalleeSavedStackSize() - 16) : 0;
+ int64_t FPOffset = isTargetDarwin(MF) ? (AFI->getCalleeSavedStackSize() - 16) : 0;
if (CombineSPBump)
FPOffset += AFI->getLocalStackSize();
@@ -1031,7 +1031,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
}
if (windowsRequiresStackProbe(MF, NumBytes)) {
- uint32_t NumWords = NumBytes >> 4;
+ uint64_t NumWords = NumBytes >> 4;
if (NeedsWinCFI) {
HasWinCFI = true;
// alloc_l can hold at most 256MB, so assume that NumBytes doesn't
@@ -1388,8 +1388,8 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF,
IsFunclet = isFuncletReturnInstr(*MBBI);
}
- int NumBytes = IsFunclet ? (int)getWinEHFuncletFrameSize(MF)
- : MFI.getStackSize();
+ int64_t NumBytes = IsFunclet ? getWinEHFuncletFrameSize(MF)
+ : MFI.getStackSize();
AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
// All calls are tail calls in GHC calling conv, and functions have no
@@ -1578,7 +1578,7 @@ void AArch64FrameLowering::emitEpilogue(MachineFunction &MF,
return;
bool NoCalleeSaveRestore = PrologueSaveSize == 0;
- int StackRestoreBytes = RedZone ? 0 : NumBytes;
+ int64_t StackRestoreBytes = RedZone ? 0 : NumBytes;
if (NoCalleeSaveRestore)
StackRestoreBytes += AfterCSRPopSize;
@@ -1670,7 +1670,7 @@ int AArch64FrameLowering::getNonLocalFrameIndexReference(
return getSEHFrameIndexOffset(MF, FI);
}
-static StackOffset getFPOffset(const MachineFunction &MF, int ObjectOffset) {
+static StackOffset getFPOffset(const MachineFunction &MF, int64_t ObjectOffset) {
const auto *AFI = MF.getInfo<AArch64FunctionInfo>();
const auto &Subtarget = MF.getSubtarget<AArch64Subtarget>();
bool IsWin64 =
@@ -1681,9 +1681,9 @@ static StackOffset getFPOffset(const MachineFunction &MF, int ObjectOffset) {
return {ObjectOffset + FixedObject + FPAdjust, MVT::i8};
}
-static StackOffset getStackOffset(const MachineFunction &MF, int ObjectOffset) {
+static StackOffset getStackOffset(const MachineFunction &MF, int64_t ObjectOffset) {
const auto &MFI = MF.getFrameInfo();
- return {ObjectOffset + (int)MFI.getStackSize(), MVT::i8};
+ return {ObjectOffset + (int64_t)MFI.getStackSize(), MVT::i8};
}
int AArch64FrameLowering::getSEHFrameIndexOffset(const MachineFunction &MF,
@@ -1700,7 +1700,7 @@ StackOffset AArch64FrameLowering::resolveFrameIndexReference(
const MachineFunction &MF, int FI, unsigned &FrameReg, bool PreferFP,
bool ForSimm) const {
const auto &MFI = MF.getFrameInfo();
- int ObjectOffset = MFI.getObjectOffset(FI);
+ int64_t ObjectOffset = MFI.getObjectOffset(FI);
bool isFixed = MFI.isFixedObjectIndex(FI);
bool isSVE = MFI.getStackID(FI) == TargetStackID::SVEVector;
return resolveFrameOffsetReference(MF, ObjectOffset, isFixed, isSVE, FrameReg,
@@ -1708,7 +1708,7 @@ StackOffset AArch64FrameLowering::resolveFrameIndexReference(
}
StackOffset AArch64FrameLowering::resolveFrameOffsetReference(
- const MachineFunction &MF, int ObjectOffset, bool isFixed, bool isSVE,
+ const MachineFunction &MF, int64_t ObjectOffset, bool isFixed, bool isSVE,
unsigned &FrameReg, bool PreferFP, bool ForSimm) const {
const auto &MFI = MF.getFrameInfo();
const auto *RegInfo = static_cast<const AArch64RegisterInfo *>(
@@ -1716,8 +1716,8 @@ StackOffset AArch64FrameLowering::resolveFrameOffsetReference(
const auto *AFI = MF.getInfo<AArch64FunctionInfo>();
const auto &Subtarget = MF.getSubtarget<AArch64Subtarget>();
- int FPOffset = getFPOffset(MF, ObjectOffset).getBytes();
- int Offset = getStackOffset(MF, ObjectOffset).getBytes();
+ int64_t FPOffset = getFPOffset(MF, ObjectOffset).getBytes();
+ int64_t Offset = getStackOffset(MF, ObjectOffset).getBytes();
bool isCSR =
!isFixed && ObjectOffset >= -((int)AFI->getCalleeSavedStackSize(MFI));
@@ -2402,7 +2402,7 @@ void AArch64FrameLowering::determineCalleeSaves(MachineFunction &MF,
unsigned NumSavedRegs = SavedRegs.count();
// The frame record needs to be created by saving the appropriate registers
- unsigned EstimatedStackSize = MFI.estimateStackSize(MF);
+ uint64_t EstimatedStackSize = MFI.estimateStackSize(MF);
if (hasFP(MF) ||
windowsRequiresStackProbe(MF, EstimatedStackSize + CSStackSize + 16)) {
SavedRegs.set(AArch64::FP);
@@ -2465,7 +2465,7 @@ void AArch64FrameLowering::determineCalleeSaves(MachineFunction &MF,
// Adding the size of additional 64bit GPR saves.
CSStackSize += 8 * (SavedRegs.count() - NumSavedRegs);
- unsigned AlignedCSStackSize = alignTo(CSStackSize, 16);
+ uint64_t AlignedCSStackSize = alignTo(CSStackSize, 16);
LLVM_DEBUG(dbgs() << "Estimated stack frame size: "
<< EstimatedStackSize + AlignedCSStackSize
<< " bytes.\n");
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.h b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
index 3ed849993684..b5719feb6b15 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
@@ -44,7 +44,7 @@ class AArch64FrameLowering : public TargetFrameLowering {
unsigned &FrameReg, bool PreferFP,
bool ForSimm) const;
StackOffset resolveFrameOffsetReference(const MachineFunction &MF,
- int ObjectOffset, bool isFixed,
+ int64_t ObjectOffset, bool isFixed,
bool isSVE, unsigned &FrameReg,
bool PreferFP, bool ForSimm) const;
bool spillCalleeSavedRegisters(MachineBasicBlock &MBB,
@@ -101,7 +101,7 @@ class AArch64FrameLowering : public TargetFrameLowering {
private:
bool shouldCombineCSRLocalStackBump(MachineFunction &MF,
- unsigned StackBumpBytes) const;
+ uint64_t StackBumpBytes) const;
int64_t estimateSVEStackObjectOffsets(MachineFrameInfo &MF) const;
int64_t assignSVEStackObjectOffsets(MachineFrameInfo &MF,
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index f952a2f62baa..1121c6f6878d 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -3109,7 +3109,7 @@ static void emitFrameOffsetAdj(MachineBasicBlock &MBB,
const unsigned MaxEncodableValue = MaxEncoding << ShiftSize;
do {
- unsigned ThisVal = std::min<unsigned>(Offset, MaxEncodableValue);
+ uint64_t ThisVal = std::min<uint64_t>(Offset, MaxEncodableValue);
unsigned LocalShiftSize = 0;
if (ThisVal > MaxEncoding) {
ThisVal = ThisVal >> ShiftSize;
diff --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
index fb49cd90b1e0..6ddb3fdb0046 100644
--- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
@@ -52,7 +52,7 @@ class AArch64FunctionInfo final : public MachineFunctionInfo {
bool HasStackFrame = false;
/// Amount of stack frame size, not including callee-saved registers.
- unsigned LocalStackSize = 0;
+ uint64_t LocalStackSize = 0;
/// The start and end frame indices for the SVE callee saves.
int MinSVECSFrameIndex = 0;
@@ -170,8 +170,8 @@ class AArch64FunctionInfo final : public MachineFunctionInfo {
bool isSplitCSR() const { return IsSplitCSR; }
void setIsSplitCSR(bool s) { IsSplitCSR = s; }
- void setLocalStackSize(unsigned Size) { LocalStackSize = Size; }
- unsigned getLocalStackSize() const { return LocalStackSize; }
+ void setLocalStackSize(uint64_t Size) { LocalStackSize = Size; }
+ uint64_t getLocalStackSize() const { return LocalStackSize; }
void setCalleeSavedStackSize(unsigned Size) {
CalleeSavedStackSize = Size;
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index 47ccef5ed83f..14f839cd4f81 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -397,7 +397,6 @@ 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.");
StackOffset SaveOffset(Offset, MVT::i8);
return isAArch64FrameOffsetLegal(*MI, SaveOffset) & AArch64FrameOffsetIsLegal;
diff --git a/llvm/test/CodeGen/AArch64/large-stack.ll b/llvm/test/CodeGen/AArch64/large-stack.ll
new file mode 100644
index 000000000000..d5d75623028a
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/large-stack.ll
@@ -0,0 +1,49 @@
+; RUN: llc < %s -mtriple=aarch64-none-linux-gnu | FileCheck %s
+
+ at .str = private unnamed_addr constant [11 x i8] c"val = %ld\0A\00", align 1
+
+; Function Attrs: noinline optnone
+define dso_local void @set_large(i64 %val) #0 {
+entry:
+ %val.addr = alloca i64, align 8
+ %large = alloca [268435456 x i64], align 8
+ %i = alloca i32, align 4
+ store i64 %val, i64* %val.addr, align 8
+ %0 = load i64, i64* %val.addr, align 8
+ %arrayidx = getelementptr inbounds [268435456 x i64], [268435456 x i64]* %large, i64 0, i64 %0
+ store i64 1, i64* %arrayidx, align 8
+ %1 = load i64, i64* %val.addr, align 8
+ %arrayidx1 = getelementptr inbounds [268435456 x i64], [268435456 x i64]* %large, i64 0, i64 %1
+ %2 = load i64, i64* %arrayidx1, align 8
+ %call = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @.str, i64 0, i64 0), i64 %2)
+ ret void
+}
+
+declare dso_local i32 @printf(i8*, ...)
+
+attributes #0 = { noinline optnone "no-frame-pointer-elim"="true" }
+
+; CHECK: stp x[[SPILL_REG1:[0-9]+]], x[[SPILL_REG2:[0-9]+]], [sp, #-[[SPILL_OFFSET1:[0-9]+]]]
+; CHECK-NEXT: str x[[SPILL_REG3:[0-9]+]], [sp, #[[SPILL_OFFSET2:[0-9]+]]]
+; CHECK-NEXT: mov x[[FRAME:[0-9]+]], sp
+; CHECK-COUNT-128: sub sp, sp, #[[STACK1:[0-9]+]], lsl #12
+; CHECK-NEXT: sub sp, sp, #[[STACK2:[0-9]+]], lsl #12
+; CHECK-NEXT: sub sp, sp, #[[STACK3:[0-9]+]]
+; CHECK: sub x[[INDEX:[0-9]+]], x[[FRAME]], #8
+; CHECK-NEXT: str x0, [x[[INDEX]]]
+; CHECK-NEXT: ldr x[[VAL1:[0-9]+]], [x[[INDEX]]]
+; CHECK-NEXT: mov x[[VAL2:[0-9]+]], #8
+; CHECK-NEXT: add x[[VAL3:[0-9]+]], sp, #8
+; CHECK-NEXT: madd x[[VAL1]], x[[VAL1]], x[[VAL2]], x[[VAL3]]
+; CHECK-NEXT: mov x[[TMP1:[0-9]+]], #1
+; CHECK-NEXT: str x[[TMP1]], [x[[VAL1]]]
+; CHECK-NEXT: ldr x[[INDEX]], [x[[INDEX]]]
+; CHECK-NEXT: mov x[[VAL4:[0-9]+]], #8
+; CHECK-NEXT: madd x[[INDEX]], x[[INDEX]], x[[VAL4]], x[[VAL3]]
+; CHECK-NEXT: ldr x1, [x[[INDEX]]
+; CHECK: bl printf
+; CHECK-COUNT-128: add sp, sp, #[[STACK1]], lsl #12
+; CHECK-NEXT: add sp, sp, #[[STACK2]], lsl #12
+; CHECK-NEXT: add sp, sp, #[[STACK3]]
+; CHECK-NEXT: ldr x[[SPILL_REG3]], [sp, #[[SPILL_OFFSET2]]]
+; CHECK-NEXT: ldp x[[SPILL_REG1]], x[[SPILL_REG2]], [sp], #[[SPILL_OFFSET1]]
More information about the llvm-commits
mailing list