This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Allow use of StackPtrOffsetReg when building spills
ClosedPublic

Authored by critson on May 12 2020, 4:49 AM.

Details

Summary

When spilling in the entry function we should be able to borrow
StackPtrOffsetReg as a last resort. This restores behaviour
removed in D75138, and fixes failures when shaders use all
SGPRs, VGPRs and spill in the entry function.

Diff Detail

Event Timeline

critson created this revision.May 12 2020, 4:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2020, 4:49 AM
foad added a subscriber: foad.May 12 2020, 5:32 AM
arsenm requested changes to this revision.May 12 2020, 5:35 AM

Needs test

This revision now requires changes to proceed.May 12 2020, 5:35 AM
critson updated this revision to Diff 263440.May 12 2020, 8:03 AM

Add test.

scott.linder accepted this revision.May 13 2020, 12:56 PM

LGTM, although CHECKs in the tests to confirm that e.g. the first involves a scavenged register and s_add, and the second involves the stack pointer and s_mov, and that this happens at the expected offset might make it easier to read and more likely to catch something that breaks this in the future. Right now it isn't clear why there are two nearly identical tests.

arsenm added inline comments.May 13 2020, 3:18 PM
llvm/test/CodeGen/AMDGPU/spill-scavenge-offset.ll
39

Needs to check something

45

A more directed tests would be better. We have a few others relying on splitting up giant vector loads, and they're some of the slowest tests in the entire lit suite

65

Can you use amdgpu-max-waves-per-eu? I'm trying to move away from the direct register limit attributes

critson updated this revision to Diff 263960.May 14 2020, 3:30 AM
  • Fix bug where offset register would not be correctly restored
  • Modify test to use smaller array (compiles much faster)
  • Add checks in test
critson marked an inline comment as done.May 14 2020, 3:33 AM
critson added inline comments.
llvm/test/CodeGen/AMDGPU/spill-scavenge-offset.ll
65

Unfortunately amdgpu-waves-per-eu does not allow us to constrain the SGPRs low enough to trigger the bug without making the test much bigger.

arsenm added inline comments.May 14 2020, 9:08 AM
llvm/test/CodeGen/AMDGPU/spill-scavenge-offset.ll
65

Don't you just need a big block of sgpr use asm to cover the difference?

critson updated this revision to Diff 264155.May 14 2020, 9:24 PM

Make test work with amdgpu-waves-per-eu.

arsenm accepted this revision.May 15 2020, 10:08 AM
This revision is now accepted and ready to land.May 15 2020, 10:08 AM
This revision was automatically updated to reflect the committed changes.