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

make_spirv_raw should handle big-endian SPIR-V binaries #3408

Closed
grovesNL opened this issue Jan 20, 2023 · 3 comments · Fixed by #3411
Closed

make_spirv_raw should handle big-endian SPIR-V binaries #3408

grovesNL opened this issue Jan 20, 2023 · 3 comments · Fixed by #3411
Labels
area: correctness We're behaving incorrectly backend: vulkan Issues with Vulkan help required We need community help to make this happen. type: bug Something isn't working

Comments

@grovesNL
Copy link
Collaborator

Description
We used to allow both little- and big-endian SPIR-V binaries in gfx/wgpu, but now it looks like the host endianness is assumed. See https://github.com/gfx-rs/gfx/blob/3da35c4339513de6a69e7794d42a63203f5820e5/src/auxil/auxil/src/lib.rs#L84 or https://github.com/ash-rs/ash/blob/8b4575086e7a6eff4d81eb5c2cf88714e56c4128/ash/src/util.rs#L127 for reference.

Repro steps
n/a

Expected vs observed behavior
Big-endian words would be passed to the Vulkan driver but this should be handled at the application level.

Extra materials
n/a

Platform
n/a

@grovesNL grovesNL added type: bug Something isn't working help required We need community help to make this happen. area: correctness We're behaving incorrectly backend: vulkan Issues with Vulkan labels Jan 20, 2023
@grovesNL grovesNL changed the title make_spirv_raw should handle big endian make_spirv_raw should handle big-endian SPIR-V binaries Jan 20, 2023
@1e1001
Copy link
Contributor

1e1001 commented Jan 20, 2023

I've got an implementation of this on my home computer, I'll submit a PR in a few hours if nobody else has by then

@1e1001
Copy link
Contributor

1e1001 commented Jan 20, 2023

Big-endian words would be passed to the Vulkan driver but this should be handled at the application level.

Oh? Does this mean we only have to check that the magic matches rather than swapping all the words?

@grovesNL
Copy link
Collaborator Author

@1e1001 oh I just meant it should be handled at somewhere above the driver-level. i.e., handling it directly in wgpu would be really useful, although it's also possible for us to require that the magic number matches if we want to require applications to provide little-endian.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly backend: vulkan Issues with Vulkan help required We need community help to make this happen. type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants