This is an archive of the discontinued LLVM Phabricator instance.

[amdgpu][lds] Use the same isKernel predicate consistently
ClosedPublic

Authored by JonChesterfield on Oct 24 2022, 7:22 AM.

Details

Summary

isKernelCC != isKernel(F->getCallingConv())
There's a test case (lower-kernel-lds.ll) that explicitly skips amdgpu_ps
so this change picks the isKernel predicate that continues to skip that
calling convention.

isKernel returns true for AMDGPU_KERNEL and SPIR_KERNEL. isKernelCC also
returns true for other calling conventions.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2022, 7:22 AM
JonChesterfield requested review of this revision.Oct 24 2022, 7:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2022, 7:22 AM
arsenm accepted this revision.Oct 31 2022, 7:31 PM

Should just delete isKernelCC, it's practically an alias for isModuleEntryFunctionCC

This revision is now accepted and ready to land.Oct 31 2022, 7:31 PM
This revision was landed with ongoing or failed builds.Nov 9 2022, 8:45 AM
This revision was automatically updated to reflect the committed changes.

OK, so this can't be cleanly factored out as in this patch, and the current behaviour is broken for IR which mixes amdgpu_kernel and shaders. LowerModuleLDS groups functions into kernel and non-kernel, and rewrites uses from non-kernels, so if shaders are reclassified as non-kernels (which this patch effectively does) then uses from shaders get rewritten to the function style lowering, which definitely won't work for them.

Difficult to fix accurately if a function is callable from a shader and from a kernel and they use different lowering strategies for LDS. I think I'm going to leave the mixed-uses broken, i.e. ignore this for now, and try to persuade graphics to adopt the same lowering as compute.