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
823

Avoid hardcoding by checking SIRegisterInfo::getReturnAddressReg?

1283

Register

1548

Register

1586

FIXME what?

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

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
855

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

860

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
143

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

660

Needs a comment explaining what this does.

716

Needs a comment explaining what this does.

1232–1233

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

1283

Needs a comment.

1482

Needs a comment.

1546

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.