This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] For PAL, make sure Scratch Buffer Descriptor do not clobber GIT pointer
ClosedPublic

Authored by RamNalamothu on Apr 29 2020, 2:05 AM.

Details

Summary

[AMDGPU] For PAL, make sure Scratch Buffer Descriptor do not clobber GIT pointer

Since SRSRC has alignment requirements, first find non GIT pointer clobbered registers for SRSRC and then if those registers clobber preloaded Scratch Wave Offset register, copy the Scratch Wave Offset register to a free SGPR.

Diff Detail

Event Timeline

RamNalamothu created this revision.Apr 29 2020, 2:05 AM
foad added a comment.Apr 29 2020, 3:57 AM

I have tested this on a downstream test case that started failing after D75138, and I can confirm that it fixes that problem. Thanks!

arsenm added inline comments.Apr 29 2020, 9:30 AM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
306

Hoist getGITPtrLoReg out of the loop

342

!PreloadedScratchWaveOffsetReg

386

Don't need = NoRegister

395

Hoist getGITPtrLoReg out of the loop. Also this should be redundant with the isAmdPal check?

405

Don't need the != NoRegister

llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
446

return Register()

447

Register, not auto

llvm/test/CodeGen/AMDGPU/SRSRC-GIT-clobber-check.ll
1 ↗(On Diff #260863)

If you're only going to check MIR, should use -stop-after and directly check the output.

A MIR test that only runs PEI is also probably more stable for this

6–8 ↗(On Diff #260863)

Negative checks are going to be really fragile here

Address review comments.

RamNalamothu marked 7 inline comments as done.Apr 30 2020, 11:22 AM
arsenm added inline comments.May 1 2020, 11:36 AM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
491

Extra parentheses

llvm/test/CodeGen/AMDGPU/SRSRC-GIT-clobber-check.mir
2

Comment seems to be a lie. The label and some instructions are missing

10

Dead "#0"

29

Odd that we ended up with a def and implicit def of the same register

Rebase and address feedback.

RamNalamothu marked 6 inline comments as done.May 4 2020, 9:08 AM
RamNalamothu added inline comments.
llvm/test/CodeGen/AMDGPU/SRSRC-GIT-clobber-check.mir
2

Missed to clean it up from the update_mir_test_checks.py o/p.

29

Not being introduced by the current changes.

If that's a preexisting bug, will be looked separately. @scott.linder FYI

RamNalamothu marked 2 inline comments as done.May 4 2020, 9:14 AM
arsenm accepted this revision.May 5 2020, 4:43 PM
This revision is now accepted and ready to land.May 5 2020, 4:43 PM