Support allocation of huge stack frame(>2g) on PPC64.
For ELFv2 ABI on Linux, quoted from the spec 2.2.3.1 General Stack Frame Requirements
There is no maximum stack frame size defined.
On AIX, XL allows such huge frame.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
Do we have a practical requirement for such a thing? At the very least, I believe we will need code in FI elimination to handle these absurd frame sizes and the test case should show accesses to the stack both before and after the huge allocation.
Comment Actions
I'm still testing the patch with variant workloads to insure corner cases are handled correctly.
Comment Actions
Tested it with test-suite and other benchmarks on AIX and Linux, no regression found.
| llvm/lib/Target/PowerPC/PPCFrameLowering.cpp | ||
|---|---|---|
| 629 | You remove this here but this pre-condition still holds for 32-bit mode. Why not change the condition to only trap in 32-bit mode rather than never? | |
| llvm/lib/Target/PowerPC/PPCInstrInfo.cpp | ||
| 3242 | "Receiver" sounds strange. "Target" seems like a more common name. | |
| 3250 | I am not suggesting we need to re-implement optimal constant materialization here, but it seems trivially easy to only emit the ORI variants only if the immediate is non-zero. | |
| llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp | ||
| 1553 | Why is this repeated here rather than calling materializeImmPostRA() as you do in other places? | |
| llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp | ||
|---|---|---|
| 1553 | This is subtle, I've checked the commit https://github.com/llvm/llvm-project/commit/d8a423cd71bb2a9b7666fd58fc82db16b4666d1d of the code above. It intends to keep variables for Registers SSA-like but materializeImmPostRA violates this rule. | |
| llvm/lib/Target/PowerPC/PPCInstrInfo.cpp | ||
|---|---|---|
| 3244 | Can you add the expected pattern to the FIXME for the not optimal pattern? | |
| llvm/test/CodeGen/PowerPC/huge-frame-call.ll | ||
| 24 | Maybe we can avoid these cfi related instructions by adding nounwind to the function. Call frame is not a test point for this patch, right? | |
Comment Actions
This LGTM.
Let's wait for @nemanjai 's comments/approval since he requested change to this patch.