This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Stay in WQM for non-intrinsic stores
ClosedPublic

Authored by nhaehnle on Jul 22 2016, 7:01 AM.

Details

Summary

Two types of stores are possible in pixel shaders: stores to memory that are
explicitly requested at the API level, and stores that are an implementation
detail of register spilling or lowering of arrays.

For the first kind of store, we must ensure that helper pixels have no effect
and hence WQM must be disabled. The second kind of store must always be
executed, because the written value may be loaded again in a way that is
relevant for helper pixels as well -- and there are no externally visible
effects anyway.

This is a candidate for the 3.9 release branch.

Diff Detail

Event Timeline

nhaehnle updated this revision to Diff 65070.Jul 22 2016, 7:01 AM
nhaehnle retitled this revision from to AMDGPU: Stay in WQM for non-intrinsic stores.
nhaehnle updated this object.
nhaehnle added a subscriber: llvm-commits.
arsenm added inline comments.Aug 2 2016, 10:50 AM
lib/Target/AMDGPU/SIInstrFormats.td
45

Period

test/CodeGen/AMDGPU/skip-if-dead.ll
379

Why does this need to change? I think the point of this was just to have a volatile operation that won't be optimized in any way

test/CodeGen/AMDGPU/wqm.ll
49–53

Should there be a copy that uses the buffer.store and the regular store?

385

I think these should check the full buffer for the offen to make sure this is a scratch access

403

You should make these volatile. I'm surprised SROA isn't killing this alloca as is for you

nhaehnle updated this revision to Diff 66511.Aug 2 2016, 11:41 AM
nhaehnle marked 3 inline comments as done.

Addressing review comments.

arsenm accepted this revision.Aug 2 2016, 12:29 PM
arsenm edited edge metadata.

LGTM. Could se another test that uses the _exact atomics

This revision is now accepted and ready to land.Aug 2 2016, 12:29 PM
This revision was automatically updated to reflect the committed changes.