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

[ci] Specify -HV 2018 to dxc #2447

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

fornwall
Copy link
Contributor

@fornwall fornwall commented Aug 19, 2023

DX Compiler release for August 2023 was just released, and that made HLSL 2021 the default, breaking the CI build.

We currently generate hlsl that is not valid HLSL 2021, as there binary logic operators can only be used with scalars, so we need to change generation:

-  doit = vec && (ShouldIDoIt(vec) || !ShouldINotDoIt(vec));
+  doit = and(vec, or(ShouldIDoIt(vec), !ShouldINotDoIt(vec)));

While possible (changing countLeadingZeros() and select() writing is enough to pass CI), fxc does not support HLSL 2021 (and the and(), or() and select() intrinsics are only available in HLSL 2021).

So this PR specifies -HV 2018 to dxc, which was the previous default version.

@ErichDonGubler
Copy link
Member

LGTM, minus the nit I left. 😊

This avoids breaking the build with the latest release of dxc, which
made HLSL the default.
@fornwall
Copy link
Contributor Author

fornwall commented Aug 19, 2023

LGTM, minus the nit I left. 😊

Thanks! I don't see a nit comment, but was it perhaps the vim .swp file :)? If so, I removed it now (and proposed #2448)!

@ErichDonGubler
Copy link
Member

Yep, that was it!

@ErichDonGubler
Copy link
Member

@jimblandy: Mergeplz?

@teoxoy
Copy link
Member

teoxoy commented Aug 29, 2023

Ideally we should be forward compatible. Could you open a new issue to track this? I think we are running into this issue only for the select implementation; we can either take the language version as a parameter or inject a polyfill for vector conditions regardless of version.

@teoxoy teoxoy merged commit 1192588 into gfx-rs:master Aug 29, 2023
9 checks passed
@fornwall fornwall deleted the specify-hlsl-version-to-dxc branch August 29, 2023 18:04
@fornwall
Copy link
Contributor Author

Ideally we should be forward compatible. Could you open a new issue to track this?

Created gfx-rs/wgpu#4498 for that!

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