This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Simplify AMDGPUAnnotateUniformValues::visitLoadInst
ClosedPublic

Authored by foad on Feb 3 2022, 7:32 AM.

Details

Summary

Always set uniform metadata on the pointer if it is an instruction, but
otherwise do not bother to create a trivial getelementptr instruction,
because AMDGPUInstrInfo::isUniformMMO can already detect that various
non-instruction pointers are uniform.

Most of the test case churn is from tests that used undef as a pointer,
which AMDGPUInstrInfo::isUniformMMO treats as uniform.

Diff Detail

Event Timeline

foad created this revision.Feb 3 2022, 7:32 AM
foad requested review of this revision.Feb 3 2022, 7:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2022, 7:32 AM
arsenm accepted this revision.Feb 3 2022, 7:36 AM

LGTM but I'm surprised there aren't any IR tests to update

This revision is now accepted and ready to land.Feb 3 2022, 7:36 AM
foad updated this revision to Diff 405650.Feb 3 2022, 8:15 AM

Remove unused noClobberClones and rebase.

foad added a comment.Feb 3 2022, 8:16 AM

I'm surprised there aren't any IR tests to update

I pushed a change to noclobber-barrier.ll to show the insertion of trivial getelementptrs, and then rebased this patch to show them going away again.

arsenm accepted this revision.Feb 3 2022, 8:18 AM
This revision was landed with ongoing or failed builds.Feb 3 2022, 8:28 AM
This revision was automatically updated to reflect the committed changes.

We could even use a second target flag and mark loads as uniform directly.

foad added a comment.Feb 3 2022, 12:04 PM

We could even use a second target flag and mark loads as uniform directly.

Sure, but I don't see a need to do that. "Uniform" really is a property of the pointer value, not of a particular load.

We could even use a second target flag and mark loads as uniform directly.

Sure, but I don't see a need to do that. "Uniform" really is a property of the pointer value, not of a particular load.

Agree, but we do not use that. Instead we are inspecting instructions in the isUniformMMO. I am just wondering if we miss some scalar cases that way. Maybe not and it is not needed.

We could even use a second target flag and mark loads as uniform directly.

Sure, but I don't see a need to do that. "Uniform" really is a property of the pointer value, not of a particular load.

Agree, but we do not use that. Instead we are inspecting instructions in the isUniformMMO. I am just wondering if we miss some scalar cases that way. Maybe not and it is not needed.

I did a quick test. Looks like it is not needed, at least it did not find anything else to scalarize in our lit tests. Nevermind I guess.