[llvm] [AMDGPU] Assert if stack grows downwards. (PR #119888)
Sergei Barannikov via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 14 05:03:58 PST 2024
================
@@ -4041,17 +4041,15 @@ SDValue SITargetLowering::lowerDYNAMIC_STACKALLOCImpl(SDValue Op,
Chain = SP.getValue(1);
MaybeAlign Alignment = cast<ConstantSDNode>(Tmp3)->getMaybeAlignValue();
const TargetFrameLowering *TFL = Subtarget->getFrameLowering();
- unsigned Opc =
- TFL->getStackGrowthDirection() == TargetFrameLowering::StackGrowsUp
- ? ISD::ADD
- : ISD::SUB;
+ assert(TFL->getStackGrowthDirection() == TargetFrameLowering::StackGrowsUp &&
+ "Stack grows upwards for AMDGPU");
SDValue ScaledSize = DAG.getNode(
ISD::SHL, dl, VT, Size,
DAG.getConstant(Subtarget->getWavefrontSizeLog2(), dl, MVT::i32));
Align StackAlign = TFL->getStackAlign();
- Tmp1 = DAG.getNode(Opc, dl, VT, SP, ScaledSize); // Value
+ Tmp1 = DAG.getNode(ISD::ADD, dl, VT, SP, ScaledSize); // Value
----------------
s-barannikov wrote:
(not related to this PR, but I thought it was worth reporting)
If I'm reading correctly, the logic is broken for the case when the object alignment is greater than the stack alignment.
The `ISD::AND` below may result in an address that points below the current SP, possibly into a previously allocated object.
Consider: `SP = 40, StackAlign = 8, ScaledSize = 16, (Scaled)Alignment = 32`
```
Tmp1 = ADD 40, 16 = 56
Tmp1 = AND 56, -32 = 32
```
The new address (32) is less than the previous value of the stack pointer (40).
https://github.com/llvm/llvm-project/pull/119888
More information about the llvm-commits
mailing list