Skip to content

Conversation

@luciechoi
Copy link
Collaborator

@luciechoi luciechoi commented Sep 11, 2025

Add a folding rule for OpBitReverse on scalar and vector types.

Noticed we are missing this optimization while working on microsoft/DirectXShaderCompiler#7680

Additionally, fix various constant folding errors for OpBitCast to handle null constants as well as lower-bit to higher-bit integer conversions. Previously, it was throwing error for e.g. OpBitcast %int %v2ushort_1_null.

@luciechoi luciechoi requested a review from s-perron September 11, 2025 19:31
Copy link
Collaborator

@s-perron s-perron left a comment

Choose a reason for hiding this comment

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

The implementation looks good, but it could use more tests. Just to have a little more coverage in case someone decides to change it.

@luciechoi luciechoi changed the title Add a folding rule for OpBitReverse. spirv-opt: Add a folding rule for OpBitReverse. Sep 13, 2025
@luciechoi
Copy link
Collaborator Author

luciechoi commented Sep 13, 2025

The implementation looks good, but it could use more tests. Just to have a little more coverage in case someone decides to change it.

Thank you, I added more test cases and modified the folding logic to handle null constants (or PLMK if it's better to fold them as well). We can modify the util, but this is also used for folding BitCast, where I'm not sure about the exact behavior for OpConstantNull.

@luciechoi luciechoi requested a review from s-perron September 13, 2025 03:01
Copy link
Collaborator

@s-perron s-perron left a comment

Choose a reason for hiding this comment

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

I think this look good now. You can merge when you think it is ready.

@luciechoi
Copy link
Collaborator Author

Thank you for the thorough review, will merge now.

@luciechoi luciechoi merged commit f3b6c44 into KhronosGroup:main Sep 23, 2025
21 of 22 checks passed
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