This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Invert ABI attribute handling
ClosedPublic

Authored by arsenm on Aug 16 2021, 1:29 PM.

Details

Summary

Previously we assumed all callable functions did not need any
implicitly passed inputs, and added attributes to functions to
indicate when they were necessary. Requiring attributes for
correctness is pretty ugly, and it makes supporting indirect and
external calls more complicated.

This inverts the direction of the attributes, so an undecorated
function is assumed to need all implicit imputs. This enables
AMDGPUAttributor by default to mark when functions are proven to not
need a given input. This strips the equivalent functionality from the
legacy AMDGPUAnnotateKernelFeatures pass.

However, AMDGPUAnnotateKernelFeatures is not fully removed at this
point although it should be in the future. It is still necessary for
the two hacky amdgpu-calls and amdgpu-stack-objects attributes, which
would be better served by a trivial analysis on the IR during
selection. Additionally, AMDGPUAnnotateKernelFeatures still
redundantly handles the uniform-work-group-size attribute to be
removed in a future commit.

At this point when not using -amdgpu-fixed-function-abi, we are still
modifying the ABI based on these newly negated attributes. In the
future, this option will be removed and the locations for implicit
inputs will always be fixed. We will then use the new attributes to
avoid passing the values when unnecessary.

Diff Detail

Event Timeline

arsenm created this revision.Aug 16 2021, 1:29 PM
arsenm requested review of this revision.Aug 16 2021, 1:29 PM
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: wdng. · View Herald Transcript

Do we have to reverse, add attributor, and remove the old code in one patch? I can't comment on the new deductions, AMD specific.

Do we have to reverse, add attributor, and remove the old code in one patch? I can't comment on the new deductions, AMD specific.

The reversing part is in D108158. This is the part to switch to using the inverted attributes, which comes along with using the attributor to add the new ones

This revision is now accepted and ready to land.Dec 1 2021, 5:55 AM
arsenm closed this revision.Dec 1 2021, 5:55 AM