Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explanation for subgroup vs dynamically uniform. #2118

Merged

Conversation

HansKristian-Work
Copy link
Contributor

Fix #2112.

@CLAassistant
Copy link

CLAassistant commented May 3, 2023

CLA assistant check
All committers have signed the CLA.

chapters/shaders.adoc Outdated Show resolved Hide resolved
chapters/shaders.adoc Outdated Show resolved Hide resolved
chapters/shaders.adoc Outdated Show resolved Hide resolved
@martty
Copy link
Contributor

martty commented May 17, 2023

Would it be possible to instead "forbid" the repacking behavior in graphics? In essence only allow implementations to pack subgroups if the shader cannot observe the difference - I think this could be a more intuitive solution.
If the perf loss is large then an extension could relax this requirement.

@HansKristian-Work
Copy link
Contributor Author

Would it be possible to instead "forbid" the repacking behavior in graphics?

Not feasible since some implementations rely on this in shipping drivers. Only option I can think of would be an extension that lets you control pack boundary.

// | DrawID = 2 | DrawID = 1 | }
// Subgroup 1: { 2, 2, 2, 2, 1, 1, 1, 1 }

uint notActuallyDynamicallyUniformAnymore =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While everything in this PR seems technically correct, it's concerning because this part suggests that you can't use a peeling loop to remove the need for the nonuniform decoration, which I think existing content relies on. Maybe we should change VUID 06274 to say is not dynamically uniform or subgroup uniform? In the long term I'd prefer to get rid of the nonuniform decoration entirely (I think there are other, implementation-dependent cases where nonuniform is misleading/wrong), but I think that's a harder change to get through.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it realistic for an application to be able to know whether they're subgroup-uniform or not in this case? Doesn't that depend on the hardware/driver and how it might pack draws?

Copy link
Contributor Author

@HansKristian-Work HansKristian-Work Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t's concerning because this part suggests that you can't use a peeling loop to remove the need for the nonuniform decoration, which I think existing content relies on.

Peeling loops to avoid nonuniformEXT are technically out of spec, since dynamically uniform is the requirement, not subgroup uniform. Not sure why it's specced like that, but it is what it is. Desktop content in the wild definitely relies on it because it works there.

I think a compiler could optimize away nonuniformEXT if the input is subgroup uniform and the hardware only cares about that kind of uniform for descriptor access.

chapters/shaders.adoc Outdated Show resolved Hide resolved
chapters/shaders.adoc Outdated Show resolved Hide resolved
chapters/shaders.adoc Outdated Show resolved Hide resolved
chapters/shaders.adoc Outdated Show resolved Hide resolved
chapters/shaders.adoc Outdated Show resolved Hide resolved
@pdaniell-nv pdaniell-nv added this to the Signed-off to Merge milestone Jul 12, 2023
@oddhack oddhack merged commit c1af931 into KhronosGroup:main Jul 19, 2023
10 checks passed

For implementations where subgroups are packed across draws, the implementation must
make sure to handle descriptor indexing correctly. From the specification's point of view,
a dynamically uniform index does not require code:NonUniform decoration,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about this? My read of the spec says the opposite:

it is mandatory for certain operands to be decorated as NonUniform if they are not guaranteed to be dynamically uniform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify the relationship between invocation group uniformity and subgroup uniformity