This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix the new atomic optimizer in pixel shaders.
ClosedPublic

Authored by sheredom on Oct 31 2018, 6:46 AM.

Details

Summary

The new atomic optimizer I previously added in D51969 did not work correctly when a pixel shader was using derivatives, and had helper lanes active.

To fix this we add an llvm.amdgcn.ps.live call that guards a branch around the entire atomic operation - ensuring that all helper lanes are inactive within the wavefront when we compute our atomic results.

I've added a test case that can cause derivatives, and exposes the problem.

Diff Detail

Event Timeline

sheredom created this revision.Oct 31 2018, 6:46 AM
arsenm added inline comments.Oct 31 2018, 11:39 AM
test/CodeGen/AMDGPU/atomic_optimizations_pixelshader.ll
2

Remove the -amdgiz's since that's long obsolete. Also remove -march since it's redundant with the triple

sheredom updated this revision to Diff 172104.Nov 1 2018, 3:25 AM

Review fixes.

sheredom marked an inline comment as done.Nov 1 2018, 3:26 AM
nhaehnle accepted this revision.Nov 5 2018, 3:02 AM
nhaehnle added inline comments.
test/CodeGen/AMDGPU/atomic_optimizations_pixelshader.ll
25

Please remove the empty lines in this test and below.

42

Why are there no check lines for GFX7LESS? What's the difference?

This revision is now accepted and ready to land.Nov 5 2018, 3:02 AM
sheredom updated this revision to Diff 172567.Nov 5 2018, 4:04 AM

Fix review comments made by @nhaehnle

sheredom marked 2 inline comments as done.Nov 5 2018, 4:05 AM
sheredom added inline comments.
test/CodeGen/AMDGPU/atomic_optimizations_pixelshader.ll
42

The opt doesn't apply < gfx8 because of lack of cross-wave opts, but I should have checked that it definitely didn't apply (so I added the GFX7LESS-NOT lines).

This revision was automatically updated to reflect the committed changes.
sheredom marked an inline comment as done.