Skip to content

Commit e2ebc82

Browse files
tmatheson-armmemfrob
authored andcommitted
[NFC][CodeGen] Tidy up TargetRegisterInfo stack realignment functions
Currently needsStackRealignment returns false if canRealignStack returns false. This means that the behavior of needsStackRealignment does not correspond to it's name and description; a function might need stack realignment, but if it is not possible then this function returns false. Furthermore, needsStackRealignment is not virtual and therefore some backends have made use of canRealignStack to indicate whether a function needs stack realignment. This patch attempts to clarify the situation by separating them and introducing new names: - shouldRealignStack - true if there is any reason the stack should be realigned - canRealignStack - true if we are still able to realign the stack (e.g. we can still reserve/have reserved a frame pointer) - hasStackRealignment = shouldRealignStack && canRealignStack (not target customisable) Targets can now override shouldRealignStack to indicate that stack realignment is required. This change will make it easier in a future change to handle the case where we need to realign the stack but can't do so (for example when the register allocator creates an aligned spill after the frame pointer has been eliminated). Differential Revision: https://reviews.llvm.org/D98716 Change-Id: Ib9a4d21728bf9d08a545b4365418d3ffe1af4d87
1 parent 1fea98b commit e2ebc82

39 files changed

+124
-143
lines changed

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -914,9 +914,12 @@ class TargetRegisterInfo : public MCRegisterInfo {
914914

915915
/// True if storage within the function requires the stack pointer to be
916916
/// aligned more than the normal calling convention calls for.
917-
/// This cannot be overriden by the target, but canRealignStack can be
918-
/// overridden.
919-
bool needsStackRealignment(const MachineFunction &MF) const;
917+
virtual bool shouldRealignStack(const MachineFunction &MF) const;
918+
919+
/// True if stack realignment is required and still possible.
920+
bool hasStackRealignment(const MachineFunction &MF) const {
921+
return shouldRealignStack(MF) && canRealignStack(MF);
922+
}
920923

921924
/// Get the offset from the referenced frame index in the instruction,
922925
/// if there is one.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1377,7 +1377,7 @@ void CodeViewDebug::beginFunctionImpl(const MachineFunction *MF) {
13771377
CurFn->CSRSize = MFI.getCVBytesOfCalleeSavedRegisters();
13781378
CurFn->FrameSize = MFI.getStackSize();
13791379
CurFn->OffsetAdjustment = MFI.getOffsetAdjustment();
1380-
CurFn->HasStackRealignment = TRI->needsStackRealignment(*MF);
1380+
CurFn->HasStackRealignment = TRI->hasStackRealignment(*MF);
13811381

13821382
// For this function S_FRAMEPROC record, figure out which codeview register
13831383
// will be the frame pointer.

llvm/lib/CodeGen/GCRootLowering.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,8 +317,8 @@ bool GCMachineCodeAnalysis::runOnMachineFunction(MachineFunction &MF) {
317317
// size, we use UINT64_MAX to represent this.
318318
const MachineFrameInfo &MFI = MF.getFrameInfo();
319319
const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
320-
const bool DynamicFrameSize = MFI.hasVarSizedObjects() ||
321-
RegInfo->needsStackRealignment(MF);
320+
const bool DynamicFrameSize =
321+
MFI.hasVarSizedObjects() || RegInfo->hasStackRealignment(MF);
322322
FI->setFrameSize(DynamicFrameSize ? UINT64_MAX : MFI.getStackSize());
323323

324324
// Find all safe points.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,7 +1370,7 @@ bool CombinerHelper::optimizeMemcpy(MachineInstr &MI, Register Dst,
13701370
// Don't promote to an alignment that would require dynamic stack
13711371
// realignment.
13721372
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
1373-
if (!TRI->needsStackRealignment(MF))
1373+
if (!TRI->hasStackRealignment(MF))
13741374
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
13751375
NewAlign = NewAlign / 2;
13761376

@@ -1475,7 +1475,7 @@ bool CombinerHelper::optimizeMemmove(MachineInstr &MI, Register Dst,
14751475
// Don't promote to an alignment that would require dynamic stack
14761476
// realignment.
14771477
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
1478-
if (!TRI->needsStackRealignment(MF))
1478+
if (!TRI->hasStackRealignment(MF))
14791479
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
14801480
NewAlign = NewAlign / 2;
14811481

llvm/lib/CodeGen/MachineFrameInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ uint64_t MachineFrameInfo::estimateStackSize(const MachineFunction &MF) const {
173173
// value.
174174
Align StackAlign;
175175
if (adjustsStack() || hasVarSizedObjects() ||
176-
(RegInfo->needsStackRealignment(MF) && getObjectIndexEnd() != 0))
176+
(RegInfo->hasStackRealignment(MF) && getObjectIndexEnd() != 0))
177177
StackAlign = TFI->getStackAlign();
178178
else
179179
StackAlign = TFI->getTransientStackAlign();

llvm/lib/CodeGen/PrologEpilogInserter.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -877,10 +877,9 @@ void PEI::calculateFrameObjectOffsets(MachineFunction &MF) {
877877
// incoming stack pointer if a frame pointer is required and is closer
878878
// to the incoming rather than the final stack pointer.
879879
const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
880-
bool EarlyScavengingSlots = (TFI.hasFP(MF) &&
881-
TFI.isFPCloseToIncomingSP() &&
880+
bool EarlyScavengingSlots = (TFI.hasFP(MF) && TFI.isFPCloseToIncomingSP() &&
882881
RegInfo->useFPForScavengingIndex(MF) &&
883-
!RegInfo->needsStackRealignment(MF));
882+
!RegInfo->hasStackRealignment(MF));
884883
if (RS && EarlyScavengingSlots) {
885884
SmallVector<int, 2> SFIs;
886885
RS->getScavengingFrameIndices(SFIs);
@@ -1063,7 +1062,7 @@ void PEI::calculateFrameObjectOffsets(MachineFunction &MF) {
10631062
// value.
10641063
Align StackAlign;
10651064
if (MFI.adjustsStack() || MFI.hasVarSizedObjects() ||
1066-
(RegInfo->needsStackRealignment(MF) && MFI.getObjectIndexEnd() != 0))
1065+
(RegInfo->hasStackRealignment(MF) && MFI.getObjectIndexEnd() != 0))
10671066
StackAlign = TFI.getStackAlign();
10681067
else
10691068
StackAlign = TFI.getTransientStackAlign();

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6243,7 +6243,7 @@ static SDValue getMemcpyLoadsAndStores(SelectionDAG &DAG, const SDLoc &dl,
62436243
// Don't promote to an alignment that would require dynamic stack
62446244
// realignment.
62456245
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
6246-
if (!TRI->needsStackRealignment(MF))
6246+
if (!TRI->hasStackRealignment(MF))
62476247
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
62486248
NewAlign = NewAlign / 2;
62496249

llvm/lib/CodeGen/StackMaps.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ void StackMaps::recordStackMapOpers(const MCSymbol &MILabel,
511511
const MachineFrameInfo &MFI = AP.MF->getFrameInfo();
512512
const TargetRegisterInfo *RegInfo = AP.MF->getSubtarget().getRegisterInfo();
513513
bool HasDynamicFrameSize =
514-
MFI.hasVarSizedObjects() || RegInfo->needsStackRealignment(*(AP.MF));
514+
MFI.hasVarSizedObjects() || RegInfo->hasStackRealignment(*(AP.MF));
515515
uint64_t FrameSize = HasDynamicFrameSize ? UINT64_MAX : MFI.getStackSize();
516516

517517
auto CurrentIt = FnInfos.find(AP.CurrentFnSym);

llvm/lib/CodeGen/TargetRegisterInfo.cpp

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -461,21 +461,13 @@ bool TargetRegisterInfo::canRealignStack(const MachineFunction &MF) const {
461461
return !MF.getFunction().hasFnAttribute("no-realign-stack");
462462
}
463463

464-
bool TargetRegisterInfo::needsStackRealignment(
465-
const MachineFunction &MF) const {
464+
bool TargetRegisterInfo::shouldRealignStack(const MachineFunction &MF) const {
466465
const MachineFrameInfo &MFI = MF.getFrameInfo();
467466
const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
468467
const Function &F = MF.getFunction();
469-
Align StackAlign = TFI->getStackAlign();
470-
bool requiresRealignment = ((MFI.getMaxAlign() > StackAlign) ||
471-
F.hasFnAttribute(Attribute::StackAlignment));
472-
if (F.hasFnAttribute("stackrealign") || requiresRealignment) {
473-
if (canRealignStack(MF))
474-
return true;
475-
LLVM_DEBUG(dbgs() << "Can't realign function's stack: " << F.getName()
476-
<< "\n");
477-
}
478-
return false;
468+
return F.hasFnAttribute("stackrealign") ||
469+
(MFI.getMaxAlign() > TFI->getStackAlign()) ||
470+
F.hasFnAttribute(Attribute::StackAlignment);
479471
}
480472

481473
bool TargetRegisterInfo::regmaskSubsetEqual(const uint32_t *mask0,

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ bool AArch64FrameLowering::homogeneousPrologEpilog(
252252
// Bail on stack adjustment needed on return for simplicity.
253253
const MachineFrameInfo &MFI = MF.getFrameInfo();
254254
const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
255-
if (MFI.hasVarSizedObjects() || RegInfo->needsStackRealignment(MF))
255+
if (MFI.hasVarSizedObjects() || RegInfo->hasStackRealignment(MF))
256256
return false;
257257
if (Exit && getArgumentPopSize(MF, *Exit))
258258
return false;
@@ -363,7 +363,7 @@ bool AArch64FrameLowering::hasFP(const MachineFunction &MF) const {
363363
return true;
364364
if (MFI.hasVarSizedObjects() || MFI.isFrameAddressTaken() ||
365365
MFI.hasStackMap() || MFI.hasPatchPoint() ||
366-
RegInfo->needsStackRealignment(MF))
366+
RegInfo->hasStackRealignment(MF))
367367
return true;
368368
// With large callframes around we may need to use FP to access the scavenging
369369
// emergency spillslot.
@@ -616,7 +616,7 @@ bool AArch64FrameLowering::canUseAsPrologue(
616616
const AArch64RegisterInfo *RegInfo = Subtarget.getRegisterInfo();
617617

618618
// Don't need a scratch register if we're not going to re-align the stack.
619-
if (!RegInfo->needsStackRealignment(*MF))
619+
if (!RegInfo->hasStackRealignment(*MF))
620620
return true;
621621
// Otherwise, we can use any block as long as it has a scratch register
622622
// available.
@@ -678,7 +678,7 @@ bool AArch64FrameLowering::shouldCombineCSRLocalStackBump(
678678
if (MFI.hasVarSizedObjects())
679679
return false;
680680

681-
if (RegInfo->needsStackRealignment(MF))
681+
if (RegInfo->hasStackRealignment(MF))
682682
return false;
683683

684684
// This isn't strictly necessary, but it simplifies things a bit since the
@@ -1375,7 +1375,7 @@ void AArch64FrameLowering::emitPrologue(MachineFunction &MF,
13751375
if (NumBytes) {
13761376
// Alignment is required for the parent frame, not the funclet
13771377
const bool NeedsRealignment =
1378-
!IsFunclet && RegInfo->needsStackRealignment(MF);
1378+
!IsFunclet && RegInfo->hasStackRealignment(MF);
13791379
unsigned scratchSPReg = AArch64::SP;
13801380

13811381
if (NeedsRealignment) {
@@ -1981,13 +1981,13 @@ StackOffset AArch64FrameLowering::resolveFrameOffsetReference(
19811981
// Argument access should always use the FP.
19821982
if (isFixed) {
19831983
UseFP = hasFP(MF);
1984-
} else if (isCSR && RegInfo->needsStackRealignment(MF)) {
1984+
} else if (isCSR && RegInfo->hasStackRealignment(MF)) {
19851985
// References to the CSR area must use FP if we're re-aligning the stack
19861986
// since the dynamically-sized alignment padding is between the SP/BP and
19871987
// the CSR area.
19881988
assert(hasFP(MF) && "Re-aligned stack must have frame pointer");
19891989
UseFP = true;
1990-
} else if (hasFP(MF) && !RegInfo->needsStackRealignment(MF)) {
1990+
} else if (hasFP(MF) && !RegInfo->hasStackRealignment(MF)) {
19911991
// If the FPOffset is negative and we're producing a signed immediate, we
19921992
// have to keep in mind that the available offset range for negative
19931993
// offsets is smaller than for positive ones. If an offset is available
@@ -2029,9 +2029,10 @@ StackOffset AArch64FrameLowering::resolveFrameOffsetReference(
20292029
}
20302030
}
20312031

2032-
assert(((isFixed || isCSR) || !RegInfo->needsStackRealignment(MF) || !UseFP) &&
2033-
"In the presence of dynamic stack pointer realignment, "
2034-
"non-argument/CSR objects cannot be accessed through the frame pointer");
2032+
assert(
2033+
((isFixed || isCSR) || !RegInfo->hasStackRealignment(MF) || !UseFP) &&
2034+
"In the presence of dynamic stack pointer realignment, "
2035+
"non-argument/CSR objects cannot be accessed through the frame pointer");
20352036

20362037
if (isSVE) {
20372038
StackOffset FPOffset =
@@ -2041,10 +2042,9 @@ StackOffset AArch64FrameLowering::resolveFrameOffsetReference(
20412042
StackOffset::get(MFI.getStackSize() - AFI->getCalleeSavedStackSize(),
20422043
ObjectOffset);
20432044
// Always use the FP for SVE spills if available and beneficial.
2044-
if (hasFP(MF) &&
2045-
(SPOffset.getFixed() ||
2046-
FPOffset.getScalable() < SPOffset.getScalable() ||
2047-
RegInfo->needsStackRealignment(MF))) {
2045+
if (hasFP(MF) && (SPOffset.getFixed() ||
2046+
FPOffset.getScalable() < SPOffset.getScalable() ||
2047+
RegInfo->hasStackRealignment(MF))) {
20482048
FrameReg = RegInfo->getFrameRegister(MF);
20492049
return FPOffset;
20502050
}

0 commit comments

Comments
 (0)