This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Disable AMDGPUPromoteAlloca pass for shader calling conventions.
ClosedPublic

Authored by bnieuwenhuizen on May 27 2016, 5:54 AM.

Details

Summary

The work item intrinsics are not available for the shader
calling conventions. And even if we did hook them up most
shader stages haves some extra restrictions on the amount
of available LDS.

Diff Detail

Event Timeline

bnieuwenhuizen retitled this revision from to AMDGPU: Disable AMDGPU for shader calling conventions..
bnieuwenhuizen updated this object.
bnieuwenhuizen added a subscriber: llvm-commits.

The title needs to be fixed. Apart from that, LGTM.

bnieuwenhuizen retitled this revision from AMDGPU: Disable AMDGPU for shader calling conventions. to AMDGPU: Disable AMDGPUPromoteAlloca pass for shader calling conventions..May 31 2016, 3:19 AM
arsenm edited edge metadata.May 31 2016, 9:57 AM

This at least needs a comment for why. What are the additional restrictions? This seems like it wouldn't be hard to fix

The problem is that Mesa currently lies about which data lives in LDS, in particular for tessellation shader inputs and outputs. A proper fix would first need to take care of that.

In general it's unclear to me how Mesa and LLVM should communicate about how LDS is used. Can we get a guarantee e.g. that LLVM-reserved LDS memory will always be placed after any LDS object in the initial IR?

I vote to fix the current issue for now (though yes, tests sound like a good idea :)) while we figure out the LDS allocation issues.

Rather than skipping the entire pass, can we just disable the promotion to LDS? Lowering alloca to vectors is still useful.

The problem is that Mesa currently lies about which data lives in LDS, in particular for tessellation shader inputs and outputs. A proper fix would first need to take care of that.

In general it's unclear to me how Mesa and LLVM should communicate about how LDS is used. Can we get a guarantee e.g. that LLVM-reserved LDS memory will always be placed after any LDS object in the initial IR?

I vote to fix the current issue for now (though yes, tests sound like a good idea :)) while we figure out the LDS allocation issues.

The order allocated is the reverse use order in the function, so it could be either order. This needs to be fixed for other reasons too, but is a pain

bnieuwenhuizen edited edge metadata.

Added a test & comment and moved the check so that only the promote to LDS is disabled.

An example of extra conditions of LDS usage for other stages is the PS needing implicit LDS
space for interpolation inputs. Furthermore some shader have no LDS_SIZE register and I am
not sure whether we can allocate some in all cases (e.g. VS has no dedicated field, but can
we allocate using the LS LDS_SIZE even if the LS does not run?)

Could someone commit this?

It should rebase cleanly and still pass the testsuite.

This revision was automatically updated to reflect the committed changes.