Skip to content

[CodeGen] Get stack alignment from TargetFrameLowering #105477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[CodeGen] Get stack alignment from TargetFrameLowering #105477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96f407c
[CodeGen] Get stack alignment from TargetFrameLowering (NFCI)
s-barannikov Aug 21, 2024
e76dd71
Update failing tests
s-barannikov Aug 21, 2024
5b0c543
More precise check
s-barannikov Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9133,9 +9133,10 @@ LegalizerHelper::lowerMemcpy(MachineInstr &MI, Register Dst, Register Src,
// Don't promote to an alignment that would require dynamic stack
// realignment.
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
if (!TRI->hasStackRealignment(MF))
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
NewAlign = NewAlign.previous();
if (!TRI->hasStackRealignment(MF)) {
const TargetFrameLowering *TFL = MF.getSubtarget().getFrameLowering();
NewAlign = std::min(NewAlign, std::max(Alignment, TFL->getStackAlign()));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of code duplication. Why not move this code into a function and use it everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I'll try to find a common place, or maybe you can suggest one?


if (NewAlign > Alignment) {
Alignment = NewAlign;
Expand Down Expand Up @@ -9241,9 +9242,10 @@ LegalizerHelper::lowerMemmove(MachineInstr &MI, Register Dst, Register Src,
// Don't promote to an alignment that would require dynamic stack
// realignment.
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
if (!TRI->hasStackRealignment(MF))
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
NewAlign = NewAlign.previous();
if (NewAlign > Alignment && !TRI->hasStackRealignment(MF)) {
const TargetFrameLowering *TFL = MF.getSubtarget().getFrameLowering();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we assert that the TFL value is the same as the datalayout? Preferably we wouldn't have 2, and only have the DL one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having two of them either, but AIUI the one from DataLayout is more of a heuristic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TFL one is also another fixed constant, just set in a different place

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we assert that the TFL value is the same as the datalayout?

It appears they values are different on AMDGPU. Data layout string specifies "S32" (4 bytes), but TFL value is 16 bytes. I couldn't conclude if this is intended or an oversight.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not intended. I don't know what the DataLayout one is even used for, it should change to match

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DataLayout one is used in similar scenarios, but in middle end passes. I.e. when promoting alloca alignment.

NewAlign = std::min(NewAlign, TFL->getStackAlign());
}

if (NewAlign > Alignment) {
Alignment = NewAlign;
Expand Down
21 changes: 12 additions & 9 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7893,9 +7893,10 @@ static SDValue getMemcpyLoadsAndStores(SelectionDAG &DAG, const SDLoc &dl,
// realignment which may conflict with optimizations such as tail call
// optimization.
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
if (!TRI->hasStackRealignment(MF))
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
NewAlign = NewAlign.previous();
if (!TRI->hasStackRealignment(MF)) {
const TargetFrameLowering *TFL = MF.getSubtarget().getFrameLowering();
NewAlign = std::min(NewAlign, std::max(Alignment, TFL->getStackAlign()));
}

if (NewAlign > Alignment) {
// Give the stack frame object a larger alignment if needed.
Expand Down Expand Up @@ -8088,9 +8089,10 @@ static SDValue getMemmoveLoadsAndStores(SelectionDAG &DAG, const SDLoc &dl,
// realignment which may conflict with optimizations such as tail call
// optimization.
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
if (!TRI->hasStackRealignment(MF))
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
NewAlign = NewAlign.previous();
if (!TRI->hasStackRealignment(MF)) {
const TargetFrameLowering *TFL = MF.getSubtarget().getFrameLowering();
NewAlign = std::min(NewAlign, std::max(Alignment, TFL->getStackAlign()));
}

if (NewAlign > Alignment) {
// Give the stack frame object a larger alignment if needed.
Expand Down Expand Up @@ -8206,9 +8208,10 @@ static SDValue getMemsetStores(SelectionDAG &DAG, const SDLoc &dl,
// realignment which may conflict with optimizations such as tail call
// optimization.
const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
if (!TRI->hasStackRealignment(MF))
while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
NewAlign = NewAlign.previous();
if (!TRI->hasStackRealignment(MF)) {
const TargetFrameLowering *TFL = MF.getSubtarget().getFrameLowering();
NewAlign = std::min(NewAlign, std::max(Alignment, TFL->getStackAlign()));
}

if (NewAlign > Alignment) {
// Give the stack frame object a larger alignment if needed.
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,7 @@ static bool CC_AArch64_Custom_Block(unsigned &ValNo, MVT &ValVT, MVT &LocVT,
State.AllocateReg(Reg);
}

const Align StackAlign =
State.getMachineFunction().getDataLayout().getStackAlignment();
const Align StackAlign = Subtarget.getFrameLowering()->getStackAlign();
const Align MemAlign = ArgFlags.getNonZeroMemAlign();
Align SlotAlign = std::min(MemAlign, StackAlign);
if (!Subtarget.isTargetDarwin())
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Target/ARM/ARMCallingConv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,9 @@ static bool CC_ARM_AAPCS_Custom_Aggregate(unsigned ValNo, MVT ValVT,

// Try to allocate a contiguous block of registers, each of the correct
// size to hold one member.
auto &DL = State.getMachineFunction().getDataLayout();
const Align StackAlign = DL.getStackAlignment();
const auto &Subtarget =
State.getMachineFunction().getSubtarget<ARMSubtarget>();
const Align StackAlign = Subtarget.getFrameLowering()->getStackAlign();
const Align FirstMemberAlign(PendingMembers[0].getExtraInfo());
Align Alignment = std::min(FirstMemberAlign, StackAlign);

Expand Down Expand Up @@ -265,7 +266,7 @@ static bool CC_ARM_AAPCS_Custom_Aggregate(unsigned ValNo, MVT ValVT,
State.AllocateReg(Reg);

// Clamp the alignment between 4 and 8.
if (State.getMachineFunction().getSubtarget<ARMSubtarget>().isTargetAEABI())
if (Subtarget.isTargetAEABI())
Alignment = ArgFlags.getNonZeroMemAlign() <= 4 ? Align(4) : Align(8);

// After the first item has been allocated, the rest are packed as tightly as
Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/Target/ARM/ARMISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2461,7 +2461,7 @@ ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,

// Since callee will pop argument stack as a tail call, we must keep the
// popped size 16-byte aligned.
Align StackAlign = DAG.getDataLayout().getStackAlignment();
Align StackAlign = Subtarget->getFrameLowering()->getStackAlign();
NumBytes = alignTo(NumBytes, StackAlign);

// SPDiff will be negative if this tail call requires more space than we
Expand Down Expand Up @@ -4711,9 +4711,8 @@ SDValue ARMTargetLowering::LowerFormalArguments(
if (canGuaranteeTCO(CallConv, TailCallOpt)) {
// The only way to guarantee a tail call is if the callee restores its
// argument area, but it must also keep the stack aligned when doing so.
const DataLayout &DL = DAG.getDataLayout();
StackArgSize = alignTo(StackArgSize, DL.getStackAlignment());

Align StackAlign = Subtarget->getFrameLowering()->getStackAlign();
StackArgSize = alignTo(StackArgSize, StackAlign);
AFI->setArgumentStackToRestore(StackArgSize);
}
AFI->setArgumentStackSize(StackArgSize);
Expand Down Expand Up @@ -22030,7 +22029,7 @@ Align ARMTargetLowering::getABIAlignmentForCallingConv(

// Avoid over-aligning vector parameters. It would require realigning the
// stack and waste space for no real benefit.
return std::min(ABITypeAlign, DL.getStackAlignment());
return std::min(ABITypeAlign, Subtarget->getFrameLowering()->getStackAlign());
}

/// Return true if a type is an AAPCS-VFP homogeneous aggregate or one of
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,9 +1189,9 @@ WebAssemblyTargetLowering::LowerCall(CallLoweringInfo &CLI,
if (IsVarArg && NumBytes) {
// For non-fixed arguments, next emit stores to store the argument values
// to the stack buffer at the offsets computed above.
int FI = MF.getFrameInfo().CreateStackObject(NumBytes,
Layout.getStackAlignment(),
/*isSS=*/false);
Align StackAlign = Subtarget->getFrameLowering()->getStackAlign();
int FI = MF.getFrameInfo().CreateStackObject(NumBytes, StackAlign,
/*isSpillSlot=*/false);
unsigned ValNo = 0;
SmallVector<SDValue, 8> Chains;
for (SDValue Arg : drop_begin(OutVals, NumFixedArgs)) {
Expand Down
Loading
Loading