[SYCL] Make reduction compatible with MSVC host compiler #6601
Conversation
This PR addresses two problems:
1) MSVC has a bug in handling this in default mode (fine in /permissive-
though). The issue affected compilation using MSVC as a host compiler.
Simplified description of the problem:
template <class Derived>
class Base {
using T = int;
};
template <class T>
class A : public Base<A<T>> {
// That's what we had in the codebase prior to this change. MSVC
// complains here by default, accepts in "/permissive-".
using T2 = T;
};
class Base2 {
using T = int;
};
template <class T>
class A2 : public Base2 {
using T2 = T; // That's where the error has to be emitted.
};
int main() {
A<int> a;
A2<int> a2;
return 0;
}
2) constexpr variable are part of lambda capture and result in
incompatibilities between clang device and MSVC host. As such, don't use
those when they're used inside kernel lambdas. Instead, make them
regular variable and pay the price of increased number of arguments to
the kernel.
|
OCL's is known and unrelated. |
|
for (2) mentioned in the description: What do you mean by "incompatibilities between clang device and MSVC host"? As I read that description tells me that constexpr must never be used, but that seems like a pretty severe restriction. Users can use constexpr variables in kernel/lambda. Wouldn't it be useful to ask FE experts (e.g. @erichkeane ) to take a look at this problem too? |
Can you please give a reference (file & line number) that has such a crazy construction? |
Unfortunately this is a type of implementation divergence that would be an ABI break on either side, and the standard permits both (but, I'll say, clangs implementation is more DEFENSIBLE by standard). |
|
@aelovikov-intel - In this case, the XPTI/kernel/content.cpp failure is likely related to this as the removal of the |
intel/llvm#6601 increases number of captured variable and thus number of kernel arguments for reductions. Modify the CHECKs accordingly.
You're right. Created intel/llvm-test-suite#1164, let's wait for its pre-commit to finish. |
|
I am Ok with the changes. Thank you. |
intel/llvm#6601 increases number of captured variable and thus number of kernel arguments for reductions. Modify the CHECKs accordingly.
intel#6601 increases number of captured variable and thus number of kernel arguments for reductions. Modify the CHECKs accordingly.
…-suite#1164) intel#6601 increases number of captured variable and thus number of kernel arguments for reductions. Modify the CHECKs accordingly.
This PR addresses two problems:
though). The issue affected compilation using MSVC as a host compiler.
Simplified description of the problem:
incompatibilities between clang device and MSVC host. As such, don't use
those when they're used inside kernel lambdas. Instead, make them
regular variables and pay the price of increased number of arguments to
the kernel.