-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
add spirv64-intel-unknown target triple #151549
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
base: main
Are you sure you want to change the base?
add spirv64-intel-unknown target triple #151549
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will have to verify stuff in here, for now I just copied from the vulkan triple and changed the data layout.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Great. For the runtime failure, I think I know a fix. Other than that, can you please update the rustc-dev-guide (you can do that in this PR) and add a spirv section under the offload chapter? #149202 has an exmample. It seems like for now users will have to call out to some extra binaries to process the artifacts, until LLVM get's to integrate them properly. See e.g https://github.com/llvm/llvm-project/pull/174675/changes#r2713489339, so it would be good to document the spirv pipeline. |
compiler/rustc_target/src/spec/targets/spirv64_intel_unknown.rs
Outdated
Show resolved
Hide resolved
| Arch::Msp430 => ArchKind::Msp430, | ||
| Arch::Nvptx64 => ArchKind::Nvptx, | ||
| Arch::RiscV32 | Arch::RiscV64 => ArchKind::Riscv, | ||
| Arch::SpirV => ArchKind::Spirv, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure I agree with SPIRV being SpirV in the Arch but let's be consistent here. The ArchKind::Riscv deviation was not an intentional choice, if memory serves.
| Arch::SpirV => ArchKind::Spirv, | |
| Arch::SpirV => ArchKind::SpirV, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd go with Spirv everywhere then. But that'd have to be fixed in the "upstream" PR as well.
|
|
||
| // Checks (via an assert in `compiler/rustc_ty_utils/src/abi.rs`) that the C ABI for the current | ||
| // target correctly implements `rustc_pass_indirectly_in_non_rustic_abis`. | ||
| #[cfg(not(target_arch = "spirv"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should comment on the "upstream" PR because this is present from there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw support for variadics in spirv functions landed recently llvm/llvm-project#175076
This comment has been minimized.
This comment has been minimized.
Good catch, that's where I was stuck. But that's only a problem when we're using lto I think. Why does offloading require fat lto? |
fb06f5e to
6e7fb35
Compare
This comment has been minimized.
This comment has been minimized.
|
Mostly what Johannes said in the LLVM PR, we rely on LTO for performance, although it also simplifies the implementation a bit, since we only have one llvm-ir module, so we don't have to search for functions across multiple modules. There were two groups working on getting thin-lto performance competitive, I can ask around how that's going. In the future we could look into supporting other things than fat-lto, but so far I just didn't want to spend time on features that reduce runtime perf, if we don't even have everything needed yet to write performant kernels. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #151615) made this pull request unmergeable. Please resolve the merge conflicts. |
The target name is a bit weird but it will allow offloading rust code on intel GPUs.
Tracking issue #131513
Builds upon #150851 (commits from that are included here for now), needs this merged upstream llvm/llvm-project#174675 to be actually usable.
This is work in progress, but any feedback is appreciated.
The modified tests pass on my setup but I haven't been able to run any code on the GPU yet.