This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: GFX9 GS and HS shaders always have the scratch wave offset in SGPR5
ClosedPublic

Authored by mareko on Apr 28 2017, 7:23 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

mareko created this revision.Apr 28 2017, 7:23 AM
arsenm added inline comments.Apr 28 2017, 9:25 AM
lib/Target/AMDGPU/SIFrameLowering.cpp
311–312 ↗(On Diff #97094)

This shouldn't be a recorded property of a function. Can you check MRI for whether the register has uses?

test/CodeGen/AMDGPU/scratch-simple.ll
5–7 ↗(On Diff #97094)

Copy paste from local-stack-slot-bug.ll.

It would probably be better to merge the two tests rathe than having the exact same function repeated in multiple files.

mareko added inline comments.Apr 28 2017, 11:34 AM
test/CodeGen/AMDGPU/scratch-simple.ll
5–7 ↗(On Diff #97094)

No. This patch renames local-stack-slot-bug.ll to scratch-simple.ll and adds new tests.

mareko updated this revision to Diff 97140.Apr 28 2017, 12:28 PM

Use isPhysRegUsed.

arsenm added inline comments.May 2 2017, 11:10 AM
lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
74 ↗(On Diff #97140)

I still don't think you need this. You can directly set the input SGPR5 in the constructor here

mareko updated this revision to Diff 97483.May 2 2017, 12:13 PM

Directly set SGPR5 in the SIMachineFunctionInfo constructor.

arsenm accepted this revision.May 4 2017, 10:39 AM

LGTM, except the test is much more complicated than it needs to be. Can have a small test with a volatile access to an alloca or something? These spill tests are big enough to slow down the test suite

This revision is now accepted and ready to land.May 4 2017, 10:39 AM

Yeah, we could have a simpler test, but my approach is usually to copy an existing test and modify it, so it would help if we had such a test that screams "hey I'm a simple scratch buffer test!"

This revision was automatically updated to reflect the committed changes.
llvm/trunk/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp