Page MenuHomePhabricator

[AMDGPU] Do not annotate features for graphics
ClosedPublic

Authored by sebastian-ne on Apr 29 2021, 6:52 AM.

Details

Summary

SITargetLowering::LowerFormalArguments asserts that none of these
features are used for graphics calling conventions, so
AnnotateKernelFeatures should not add them.

Diff Detail

Event Timeline

sebastian-ne created this revision.Apr 29 2021, 6:52 AM
sebastian-ne requested review of this revision.Apr 29 2021, 6:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2021, 6:52 AM
arsenm added inline comments.Apr 29 2021, 8:40 AM
llvm/test/CodeGen/AMDGPU/pal-simple-indirect-call.ll
3

The IR checks look missing except for the label

sebastian-ne added inline comments.Apr 29 2021, 8:52 AM
llvm/test/CodeGen/AMDGPU/pal-simple-indirect-call.ll
3

This test is copied from simple-indirect-call.ll (it can’t be in the same file because amdhsa does not allow amdgpu_cs functions). The one check line still checks that there are no attributes added, but I can just remove the opt test if that is not worth it.

madhur13490 added inline comments.Apr 29 2021, 9:25 AM
llvm/lib/Target/AMDGPU/AMDGPUAnnotateKernelFeatures.cpp
406–408

Please put a comment about why you "continue" when particular CC is met.

llvm/test/CodeGen/AMDGPU/pal-simple-indirect-call.ll
3

Since this patch makes changes in the pass, I think opt check is required. llc line from simple-indirect-call.ll made sure that the test compiles fine. It's up to you if you want the similar assurance.

sebastian-ne marked an inline comment as done.

Add comment

arsenm added inline comments.Apr 29 2021, 3:17 PM
llvm/test/CodeGen/AMDGPU/pal-simple-indirect-call.ll
3

Can you add a comment indicating there should be no attribute on the function? This isn't the most obvious test

sebastian-ne marked an inline comment as done.

Good point, added comments to the test.

arsenm accepted this revision.Apr 30 2021, 4:40 PM
This revision is now accepted and ready to land.Apr 30 2021, 4:40 PM
madhur13490 accepted this revision.Apr 30 2021, 5:35 PM
This revision was landed with ongoing or failed builds.May 3 2021, 1:34 AM
This revision was automatically updated to reflect the committed changes.