Skip to content

Sync #pragma shader_stage(<stage>) to -fshader-stage=<stage> - #1575

Merged
dneto0 merged 2 commits into
google:mainfrom
LBV2012-26:main
Jul 31, 2026
Merged

Sync #pragma shader_stage(<stage>) to -fshader-stage=<stage>#1575
dneto0 merged 2 commits into
google:mainfrom
LBV2012-26:main

Conversation

@LBV2012-26

Copy link
Copy Markdown
Contributor

This PR synchronizes the shader stages flags for the -fshader-stage= option with the #pragma shader_stage(<stage>) list in glslc/src/main.cc. Such as raygen, closest, etc.

@google-cla

google-cla Bot commented Jun 21, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@dneto0

dneto0 commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

I think it's a bad idea to make new conventions for shader stage names that are aliases of existing names.

Let's stick to the conventions in Glslang:

https://github.com/KhronosGroup/glslang/blob/ce138e2c2d6992b31ff4cd2e955904637785a881/README.md?plain=1#L80

Also, updates like this should come with tests.

@dneto0 dneto0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

see comments on the main part of this PR

@LBV2012-26

LBV2012-26 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

I think it's a bad idea to make new conventions for shader stage names that are aliases of existing names.

Let's stick to the conventions in Glslang:

https://github.com/KhronosGroup/glslang/blob/ce138e2c2d6992b31ff4cd2e955904637785a881/README.md?plain=1#L80

Also, updates like this should come with tests.

Thanks for your response. But it's not the new aliases. Actually it's the same as custom pragma directives of shaderc (in libshaderc_util/src/shader_stage.cc), such as #pragma shader_stage(raygen). In addition, the tesseval and tesscontrol are also not defined by glslang.
I submit this PR is not intended to define a new alias. Currently, glslc --help completely omits all ray-tracing and mesh shader stages (like rgen, rahit, etc.), even though they are fully supported in the code. I want to fix the outdated -fshader-stage help text while also standardize the behavior of the strings accepted by #pragma shader_stage() and the CLI flags.
If you still think is improper I can close this PR. But please fix the inconsistent behavior between toolchain and the commands.
Test suite I'll commit later.

@dneto0

dneto0 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Thanks for your response. But it's not the new aliases. Actually it's the same as custom pragma directives of shaderc (in libshaderc_util/src/shader_stage.cc), such as #pragma shader_stage(raygen). In addition, the tesseval and tesscontrol are also not defined by glslang. I submit this PR is not intended to define a new alias.

Ah, I see. It's been so long I forgot about the #pragma mechanism.

Currently, glslc --help completely omits all ray-tracing and mesh shader stages (like rgen, rahit, etc.), even though they are fully supported in the code. I want to fix the outdated -fshader-stage help text while also standardize the behavior of the strings accepted by #pragma shader_stage() and the CLI flags. If you still think is improper I can close this PR. But please fix the inconsistent behavior between toolchain and the commands. Test suite I'll commit later.

I'll build on top of this to add tests. (I'll push to your tree, thanks for allowing maintainers to edit).

@dneto0 dneto0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

with the new tests i added, this LGTM.
Thanks for your patience.

@dneto0
dneto0 enabled auto-merge (rebase) July 31, 2026 18:18
@dneto0
dneto0 merged commit 4cc3991 into google:main Jul 31, 2026
25 checks passed
@LBV2012-26

Copy link
Copy Markdown
Contributor Author

with the new tests i added, this LGTM. Thanks for your patience.

Sorry and it slipped my mind about adding test suit. Thank you for your review and merge.

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.

3 participants