This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Implement -amdgpu-spill-cfi-saved-regs
AcceptedPublic

Authored by scott.linder on Mar 26 2020, 12:22 PM.

Details

Summary

These spills need special CFI anyway, so implementing them directly
where CFI is emitted avoids the need to invent a mechanism to track them
from ISel.

Diff Detail

Event Timeline

scott.linder created this revision.Mar 26 2020, 12:22 PM

git clang-format

arsenm added inline comments.Mar 26 2020, 2:23 PM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
715

Avoid hardcoding by checking SIRegisterInfo::getReturnAddressReg?

1038

Register

1315

Register

1353

FIXME what?

scott.linder marked an inline comment as done.
scott.linder added inline comments.
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
1353

The hard-coded SGPR size, I meant to come back and actually check relative to the SGPR.

Rebase and address feedback.

arsenm added inline comments.May 13 2020, 9:04 AM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
716

You can just hardcode AMDGPU::sub0 instead of TRI.getSubRegFromChannel(0)

721

AMDGPU::sub1

Refer to subreg index directly

scott.linder marked 2 inline comments as done.May 14 2020, 11:53 AM

Rebase and fix position of spill for -amdgpu-spill-cfi-saved-regs

Rebase onto LLVM master. Tidy up code little bit and couple of fixes.
And also, handle cases where no free VGPR is available to spill into and we have to spill into memeory.

cameron.mcinally accepted this revision.Sep 22 2020, 1:13 PM

LGTM with moderate confidence. Does anyone see a reason that this shouldn't land?

This revision is now accepted and ready to land.Sep 22 2020, 1:13 PM
greened added inline comments.Sep 30 2020, 2:10 PM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
97

Add a comment about why this is here. What is DwordOff about?

607

Needs a comment explaining what this does.

619

Needs a comment explaining what this does.

988

Can this be simplified by splitting into multiple if statements and commented? All of the chained logical operations are difficult to parse.

1038

Needs a comment.

1247

Needs a comment.

1313

Needs a comment.

llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
428 ↗(On Diff #290938)

Needs a comment.

Rebase onto LLVM master and address feedback

Rebase onto LLVM trunk and couple of bug fixes where we need to save FP due to CFI register spills but did not save it.

Rebase onto LLVM mainline top of the tree.

Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2022, 8:39 AM
Herald added subscribers: kosarev, foad. · View Herald Transcript

Incorporated all the downstream changes.