DouglasDwyer/mach-dxcompiler-rs

Add support for other targets

Closed this issue · 4 comments

Currently this only downloads file for Windows, need to change target process to support other targets.
For reference my script in #1

This might be as simple as adding additional targets to the build script:

fn get_target_url(target: &str) -> String {
const BASE_URL: &str = "https://github.com/hexops/mach-dxcompiler/releases/download/2023.12.14%2B0b7073b.1/";
BASE_URL.to_string() + match target {
"x86_64-pc-windows-msvc" => "x86_64-windows-msvc_ReleaseFast_lib.tar.gz",
"x86_64-pc-windows-gnu" => "x86_64-windows-gnu_ReleaseFast_lib.tar.gz",
_ => panic!("Unsupported target '{target}' for mach-dxcompiler")
}
}

There are two things that we would have to verify:

  • Downloading with curl and tar works for Mac and Linux as well. If not, we have to find another way to download the binaries.
  • This project has a dependency on windows_core so that it can expose DxcCreateInstance with the appropriate Windows types. We need to ensure that windows_core builds on Mac/Linux, and if not, the signature of DxcCreateInstance needs to be updated (or gated behind a Windows-only flag).

Downloading with curl and tar works for Mac and Linux as well. If not, we have to find another way to download the binaries.

I agree, this should be fine now.

This project has a dependency on windows_core so that it can expose DxcCreateInstance with the appropriate Windows types. We need to ensure that windows_core builds on Mac/Linux, and if not, the signature of DxcCreateInstance needs to be updated (or gated behind a Windows-only flag).

I am not sure if this should be done on our side, if we have different version of windows_core with other project, it will need to build another windows_core which increases the build time. So I only expose C api binding in my work, how do you think?

I am not sure if this should be done on our side, if we have different version of windows_core with other project, it will need to build another windows_core which increases the build time

I definitely agree. On the other hand, I think it is important to expose DxcCreateInstance because it's how I implemented Mach support for WGPU! WGPU's existing codepaths for DXC already use the COM API, so it's much easier to continue with that rather than trying to use the separate Mach C API.

I think a good middle ground might be to expose DxcCreateInstance with the signature fn(*const (), *const (), *mut *mut ()). That way, consumers of this crate can still use it and we won't have a windows_core dependency (there just will be less type safety).

I think WGPU only needs a compiler pointer becausae validation is done internally, and it is easy to get IDxcCompiler3 with a pointer type cast.