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

Implement Quad Control intrinsics #5981

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

fairywreath
Copy link
Contributor

Adds quad control intrinsics introduced in SM 6.7. Parallel implementations exist in Metal and Vulkan. This change also adds adds attribute syntax to specify quad control SPIRV execution modes that is not exposed by HLSL.

SPIRV specs: quad control, maximal reconvergence
GLSL specs: quad control, maximal reconvergence
Vulkan specs: quad control, maximal reconvergence

@fairywreath fairywreath requested a review from a team as a code owner January 1, 2025 22:42
@fairywreath
Copy link
Contributor Author

I added a new functional test which requires the quad control extensions to be enabled when creating the vulkan device. This change is reflected here shader-slang/slang-rhi#124.

//

//@public:
/// Returns true if <expr> is true in any lane of the current quad.
Copy link
Collaborator

Choose a reason for hiding this comment

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

-> expr for better documentation rendering results.

{
m_writer->emit("layout(quad_derivatives) in;\n");
}
if (irFunc->findDecoration<IRRequireFullQuadsDecoration>())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be moved to fragment shader specific handling logic in the switch statement above since it is only valid on fragment shaders.

@@ -1470,6 +1470,19 @@ void GLSLSourceEmitter::emitEntryPointAttributesImpl(
default:
break;
}

if (irFunc->findDecoration<IRQuadDerivativesDecoration>())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is starting to get too many if (findDecoration) blocks, note that each one of them is a linear search, we may just use one single for loop to interate and test the op code of all decorations.

IRBuilder builder(module);
builder.setInsertInto(entryPointFunc);

for (auto globalInst : module->getGlobalInsts())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be traversing the call graph of each EntryPoint, otherwise we will be marking all entrypoints with the decorations if any one function in the module has the opcode.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See buildEntryPointReferenceGraph, it would be best if we can perform this logic in a place where the reference graph is already available.

@fairywreath fairywreath requested a review from csyonghe January 4, 2025 00:39
csyonghe
csyonghe previously approved these changes Jan 4, 2025
Copy link
Collaborator

@csyonghe csyonghe left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@fairywreath
Copy link
Contributor Author

@csyonghe Can you merge shader-slang/slang-rhi#124 so I can update the slang-rhi submodule required to run the new functional test? Thanks!

@csyonghe
Copy link
Collaborator

csyonghe commented Jan 5, 2025

Merged.

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.

2 participants