-
Notifications
You must be signed in to change notification settings - Fork 835
[SYCL] VS2026 support #21990
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
[SYCL] VS2026 support #21990
Changes from 8 commits
02705e0
1231205
2573f86
bc8bb8d
b209ccb
a5ebd4e
168383e
5c3d967
b060afa
2632a6a
e0c19e3
0fc60e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // RUN: %clang_cc1 -triple spir64-unknown-unknown -aux-triple x86_64-pc-windows-msvc \ | ||
| // RUN: -fsycl-is-device -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s | ||
|
|
||
| // When SYCL device code is compiled with a Windows-MSVC host, the device | ||
| // target (spir64) defines _M_X64 so that MSVC STL headers take the x86 | ||
| // intrinsics path. The device target feature set must correspondingly carry | ||
| // sse/sse2 so that function-level __target__ attributes (e.g. VS2026 | ||
| // <complex>'s [[gnu::target("fma")]] on _Sqr_error_x86_x64_fma) don't strip | ||
| // the baseline, which would break intrinsic calls like _mm_set_sd / _mm_store_sd. | ||
|
Comment on lines
+4
to
+9
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand this either. Why would
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. The
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I wasn't aware of the Pondering, I wonder how much success we might have in trying to convince Microsoft to accept a series of PRs that add checks for |
||
|
|
||
| #include "Inputs/sycl.hpp" | ||
|
|
||
| int main() { | ||
| sycl::queue q; | ||
| q.submit([&](sycl::handler &h) { h.single_task<class TestK>([=] {}); }); | ||
| return 0; | ||
| } | ||
|
|
||
| // CHECK: spir_kernel void @{{.*}}TestK{{.*}}() [[ATTRS:#[0-9]+]] | ||
| // CHECK: attributes [[ATTRS]] = {{.*}}"target-features"="+sse,+sse2" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised we don't have an existing test to validate |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,21 @@ | |
|
|
||
| #pragma once | ||
|
|
||
| // Provide __isa_available for MSVC device code BEFORE including STL headers. | ||
| // Must come before #include_next so our definition is seen first. | ||
| #if defined(__SYCL_DEVICE_ONLY__) && defined(_MSC_VER) | ||
| // VS2026 STL headers use __isa_available (a runtime global variable) to | ||
| // detect CPU features: `if (__isa_available >= _Stl_isa_available_avx2) ...`. | ||
| // SYCL device code cannot access host runtime globals, so provide a device- | ||
| // side definition. The VALUE of this variable only steers the STL's runtime | ||
| // feature dispatch — both branches of the dispatch compile either way. We | ||
| // pick __ISA_AVAILABLE_X86 (== 0, the baseline in <isa_availability.h>), | ||
| // which matches a spir64 device's reality (no x86 ISA), and so selects the | ||
| // STL's scalar fallback paths if these dispatches are ever reached. | ||
| extern "C" int __isa_available __attribute__((sycl_global_var)) | ||
| __attribute__((weak)) = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a VS2026 installation available to look at. How is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question. with no guard macro. pulls this in. So substituting
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like Microsoft declares I imagine this could be a problem outside of just the |
||
| #endif // defined(__SYCL_DEVICE_ONLY__) && defined(_MSC_VER) | ||
|
|
||
| // Include real STL <complex> header - the next one from the include search | ||
| // directories. | ||
| #if defined(__has_include_next) | ||
|
|
||
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.
This doesn't make sense to me. Why would we want to claim SSE and SSE2 availability for a target for which those are not relevant?
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.
It is confusing.
The
_M_X64macro is already defined for this target, which causes MSVC STL headers to take the x86 intrinsics path. ( yay! )VS2026's
<complex>introduces inline functions like_Sqr_error_x86_x64_fmaannotated[[__gnu__::__target__("fma")]], whose bodies call_mm_set_sd/_mm_store_sd. The target attribute layers fma on top of the existing baseline, it doesn't replace it, so if the baseline lacks sse/sse2, those mm* builtins fail their feature-compatibility check at parse time, even though the function is never called from device code.So we add SSE/SSE2 just to get past that check and to get things move forward again.
To my mind, since this target already claims
_M_X64then it's really a matter of being consistent. In for a penny...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 see, thanks for the detailed explanation.