feat(deno): Add DXC env var to Deno; use DXC in cts_runner#9177
feat(deno): Add DXC env var to Deno; use DXC in cts_runner#9177andyleiserson merged 4 commits intogfx-rs:trunkfrom
Conversation
7b75a1a to
4fa5959
Compare
| shader_compiler: dx12_compiler | ||
| .unwrap_or(wgpu_types::Dx12Compiler::Fxc), |
There was a problem hiding this comment.
@crowlKats I left the default alone since changing it raises a question of how to ensure DXC is available (link statically vs. require that the library is available at runtime), but it probably should be changed to DXC at some point. Let me know if you'd like to me change it in this PR or open a Deno issue for this.
(Actually I see that as of #8882 there is a new option Dx12Compiler::Auto that uses DXC if linked or available at runtime, else falls back to FXC. Maybe we should switch to that?)
There was a problem hiding this comment.
Looks like Deno's minimum Windows version is Windows 10, version 1709 (link):
NOTE: Deno requires Windows 10 version 1709, or Windows Server 2016 version 1709 and up, due to requiring IsWow64Process2.
…which means DXC should be present in all installations, because DXC was first introduced with version 1703 (link). IOW, Deno should have no need or want for FXC (modulo bugs from earlier versions of DXC, I guess).
There was a problem hiding this comment.
Note that so far, all existing releases have been via the Windows SDKs or PIX tools.
The DXC dll needs to be shipped by applications.
| webgpu:api,validation,texture,rg11b10ufloat_renderable:* | ||
|
|
||
| webgpu:shader,execution,expression,call,builtin,atan2:f16:* | ||
| fails-if(dx12) webgpu:shader,execution,expression,call,builtin,atan2:f16:* |
ErichDonGubler
left a comment
There was a problem hiding this comment.
LGTM, nothing blocking. I have some nits that I'm sure will be resolved before merging, but aren't important enough to block. 🙂
I used Conventional Comments in this review! I hope they help with clarity and tone.
| shader_compiler: dx12_compiler | ||
| .unwrap_or(wgpu_types::Dx12Compiler::Fxc), |
There was a problem hiding this comment.
Looks like Deno's minimum Windows version is Windows 10, version 1709 (link):
NOTE: Deno requires Windows 10 version 1709, or Windows Server 2016 version 1709 and up, due to requiring IsWow64Process2.
…which means DXC should be present in all installations, because DXC was first introduced with version 1703 (link). IOW, Deno should have no need or want for FXC (modulo bugs from earlier versions of DXC, I guess).
|
nitpick: The PR title communicates that the env. var. is added, but not consumed by the CTS runner in CI. suggestion: Rename PR/eventual squash-and-merge commit title to be: nitpick: A separate commit for migrating the CTS runner to DXC would have been nice (i.e., starting with |
dbcd79c to
31cd98e
Compare
- Use the env var to select DXC in cts_runner - Sync a few other env vars from `ci.yml`
31cd98e to
19d59f1
Compare
Add the environment variable
DENO_WEBGPU_DX12_COMPILERto configure the DX12 shader compiler in Deno. Use it to select DXC in cts_runner.Also syncs a few other env vars from
ci.ymltocts.yml.Connections
Fixes #9105.
Testing
Passes the currently enabled CTS tests, with one exception, filed as #9179.
Squash or Rebase? Rebase (after squashing fixup commit)