Page MenuHomePhabricator

AMDGPU: Process amdgpu.uniform on loads
AbandonedPublic

Authored by mareko on Jan 3 2018, 1:39 PM.

Details

Reviewers
arsenm
nhaehnle
Summary

Optimizations passes can remove GEP with offset=0 and amdgpu.uniform,
but they are less likely to drop amdgpu.uniform from a load.

This is the only way to have uniform loads without GEP or when GEP is
eliminated.

Event Timeline

mareko created this revision.Jan 3 2018, 1:39 PM
arsenm added inline comments.Jan 9 2018, 7:51 AM
lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp
134–135

I'm not sure I understand the problem this is solving. Why is it dropped specifically if it is 0? If the offset is constant the GEP is uniform iff the base pointer is uniform

151–152

ConstantInt::get is simpler

155

You don't need the explicit Twine, you could also preserve the original name

test/CodeGen/AMDGPU/amdgpu.uniform.ll
5

This test should run the IR pass and check that

mareko added inline comments.Jan 10 2018, 1:49 PM
lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp
134–135

If the address is in VGPRs, the offset is 0, so we need to set amdgpu.uniform when there is no GEP.

mareko added inline comments.Jan 10 2018, 1:56 PM
lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp
155

How to do it without Twine and how to preserve the original name? Thanks.

test/CodeGen/AMDGPU/amdgpu.uniform.ll
5

Do you mean running the AnnotateUniformValues pass instead of llc? It looks like the llc test does the job too.

nhaehnle added inline comments.Jan 24 2018, 8:49 AM
lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp
143–146

This makes no sense to me. amdgpu.uniform is about whether the value could differ between threads in a wave. How often the value is used further downstream is irrelevant for that.

155

Just put "" instead of Twine("").

Not sure about preserving the original name here -- we're not transforming an instruction here, we're adding an intermediate instruction.

test/CodeGen/AMDGPU/amdgpu.uniform.ll
5

Running opt -amdgpu-annotate-uniform is potentially more stable against random changes in unrelated passes.

In this particular case though, I think using llc is justified, because we do need to test that the uniform annotation actually survives through to codegen.

mareko abandoned this revision.Jan 24 2018, 9:49 AM

I've abandoned this patch for now. Please re-open if you think this patch is useful.