[PATCH] D114652: [AMDGPU] Treat call clobbered return address registers s[30:31] as callee saved
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 1 05:57:54 PST 2021
arsenm added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp:232
+ Register Reg = TRI->getReturnAddressReg(MF);
+ if (!FuncInfo->isEntryFunction() && MFI.hasCalls()) {
+ const TargetRegisterClass *RC =
----------------
RamNalamothu wrote:
> sebastian-ne wrote:
> > Why is the return address only marked as callee-save if this function has calls? I assume it can’t it be overwritten if there are no calls because it is marked as reserved?
> >
> > Coming back here after reading the tests, I think s[30:31] should also be marked as callee-save if there are no calls. It shouldn’t cause harm if there are no calls (the register is reserved anyway) and it should fix the tests.
> > Why is the return address only marked as callee-save if this function has calls? I assume it can’t it be overwritten if there are no calls because it is marked as reserved?
> >
>
> Yes.
>
> > Coming back here after reading the tests, I think s[30:31] should also be marked as callee-save if there are no calls. It shouldn’t cause harm if there are no calls (the register is reserved anyway) and it should fix the tests.
>
> AFAIK that's the eventual goal and there is a discussion to pick the ABI registers for return address from the callee saved range. And these changes should work with no/minimal changes.
>
> However, I don't think I understand how the tests will get fixed automatically if the return address registers are marked as callee saved because even the SGPR CSR save/restore happens during SGPRSpillLowering which is where I am manually adding s[30:31] to the CSR list.
I don't think this register should be reserved, which avoids the need to do this
================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:630-633
+ // The return address register is call clobbered and the CFI needs to track
+ // its location. Hence it is handled specially.
+ if (!MFI->isEntryFunction())
+ reserveRegisterTuples(Reserved, getReturnAddressReg(MF));
----------------
I don't think it's necessary to reserve this
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114652/new/
https://reviews.llvm.org/D114652
More information about the llvm-commits
mailing list