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

Bump spirv-header/tools/glslang #5834

Merged
merged 10 commits into from
Dec 12, 2024

Conversation

expipiplus1
Copy link
Collaborator

Supersedes #5815

This PR also moves us back onto upstream spirv-tools/glslang, as our branches just had a bunch of empty merges.

This uses my patched version of spirv-tools KhronosGroup/SPIRV-Tools@2b315c2 (#5914) which closes KhronosGroup/SPIRV-Tools#5913

I suggest we wait until my PR (KhronosGroup/SPIRV-Tools#5914) is merged , or KhronosGroup/SPIRV-Tools#5913 is otherwise fixed

@expipiplus1 expipiplus1 requested a review from a team as a code owner December 11, 2024 14:17
@expipiplus1 expipiplus1 added the pr: non-breaking PRs without breaking changes label Dec 11, 2024
Copy link
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

It looks like the CI tests are still failing for the same validation error.

source/slang/hlsl.meta.slang Outdated Show resolved Hide resolved
.gitmodules Outdated Show resolved Hide resolved
@jkwak-work
Copy link
Collaborator

I see that the real fix is placed on KhronosGroup repo.
KhronosGroup/SPIRV-Tools#5914

@csyonghe
Copy link
Collaborator

Closing this one because we landed #5815.

@csyonghe csyonghe closed this Dec 11, 2024
@expipiplus1 expipiplus1 reopened this Dec 12, 2024
@expipiplus1 expipiplus1 force-pushed the push-xkllnnkkpkzv branch 2 times, most recently from b93d9cf to 42b0149 Compare December 12, 2024 04:04
Note that this currently fails validation with the following error:

```
error: line 145: Result <id> from OpSampledImage instruction must not appear as operand for OpImageSampleFootprintNV, since it is not specified as taking an OpTypeSampledImage. Found result <id> '55[%sampledImage]' as an operand of <id> '56[%resultVal]'.
  %sampledImage = OpSampledImage %54 %51 %40
```

This seems to be in error as the spec for
*SPV_NV_shader_image_footprint* states that "Sampled Image must be an
object whose type is OpTypeSampledImage"

https://refined-github-html-preview.kidonng.workers.dev/KhronosGroup/SPIRV-Registry/raw/refs/heads/main/extensions/NV/SPV_NV_shader_image_footprint.html

glslang also seems to fail with the same validation error
csyonghe
csyonghe previously approved these changes Dec 12, 2024
@csyonghe
Copy link
Collaborator

This seemed to be exposing a new test failure.

@expipiplus1
Copy link
Collaborator Author

This seemed to be exposing a new test failure.

Yeah, glslang's output changed order of some decorations, easy fix

@expipiplus1 expipiplus1 merged commit 626e814 into shader-slang:master Dec 12, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect validation error for SPV_NV_shader_image_footprint's OpImageSampleFootprintNV
3 participants