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

Unit TestsFailed

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
760

Avoid hardcoding by checking SIRegisterInfo::getReturnAddressReg?

1087

Register

1370

Register

1408

FIXME what?

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

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
761

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

766

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
91

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

608

Needs a comment explaining what this does.

661

Needs a comment explaining what this does.

1037

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

1087

Needs a comment.

1304

Needs a comment.

1368

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.