[PATCH] D78811: [AMDGPU] Enable base pointer.
Scott Linder via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 28 11:17:19 PDT 2020
scott.linder added inline comments.
================
Comment at: llvm/docs/AMDGPUUsage.rst:6706
+ to access the incoming stack arguments in the function. The BP is needed
+ only when the function requie the runtime stack alignment.
----------------
s/requie/requires/
================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:807-810
+ // If we need a base pointer, set it up here. It's whatever the value of
+ // the stack pointer is at this point. Any variable size objects will be
+ // allocated after this, so we can still use the base pointer to reference
+ // locals.
----------------
Is this comment accurate anymore? I think it can go back where it was with "base pointer" just changed to "frame pointer".
A new comment for the "base pointer" would need to talk about referencing function arguments, not locals.
================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:118
+ // stack pointer, so we reserve a base pointer.
+ return needsStackRealignment(MF);
+}
----------------
Is it possible to have some mechanism for only setting up the BP when it is actually used? Looking at X86 it seems like they don't do this, so I am OK with us not doing it either, just wondering if it is feasible.
================
Comment at: llvm/test/CodeGen/AMDGPU/callee-frame-setup.ll:302
+; GCN-NEXT: s_mov_b32 s5, s34
+; GCN-NEXT: s_mov_b32 s34, s32
; GCN-NEXT: s_add_u32 s32, s32, 0x100000
----------------
This is an example of where we have dynamic realignment but don't actually use the BP to refer to a fixed index. Again, being correct is more important, but if it is feasible it seems nice to only set this up when it is used.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78811/new/
https://reviews.llvm.org/D78811
More information about the llvm-commits
mailing list