[SYCL] Change vec operators to be friends #12396
Conversation
This commit changes operators for sycl::vec to be defined like they are in the SYCL specification, i.e. friend functions instead of members. Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
steffenlarsen
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| // RUN: %{run} %t.out | ||
|
|
||
| // This test currently fails on AMD HIP due to an unresolved memcmp function. | ||
| // XFAIL: hip_amd |
There was a problem hiding this comment.
@npmiller - It seems that this test fails on AMD HIP due to memcmp. I suspect it might be the vector comparison operators, but I have not reproduced it locally. Once this is merged I will open an issue for it.
There was a problem hiding this comment.
Is that a stdlib memcmp? We haven't added libcxx support for AMD yet so that would make sense.
There was a problem hiding this comment.
The error was:
# .---command stderr------------
# | lld: error: undefined hidden symbol: memcmp
# | >>> referenced by lto.tmp:(typeinfo name for main::'lambda163'(sycl::_V1::handler&)::operator()(sycl::_V1::handler&) const::'lambda'())
# | >>> referenced by lto.tmp:(typeinfo name for main::'lambda163'(sycl::_V1::handler&)::operator()(sycl::_V1::handler&) const::'lambda'())
# | >>> referenced by lto.tmp:(typeinfo name for main::'lambda263'(sycl::_V1::handler&)::operator()(sycl::_V1::handler&) const::'lambda'())
# | >>> referenced 1 more times
# | llvm-foreach:
# | clang++: error: amdgcn-link command failed with exit code 1 (use -v to see invocation)
# `-----------------------------
There was a problem hiding this comment.
Yeah that sounds like it, it's fine to keep it as XFAIL
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
Friendly ping @intel/llvm-reviewers-runtime @cperkinsintel |
1 similar comment
|
Friendly ping @intel/llvm-reviewers-runtime @cperkinsintel |
sycl/include/sycl/types.hpp
Outdated
| } | ||
| #endif // defined(__INTEL_PREVIEW_BREAKING_CHANGES) | ||
|
|
||
| #if !defined(__INTEL_PREVIEW_BREAKING_CHANGES) |
There was a problem hiding this comment.
we are trying to avoid using #else for long blocks. It makes the closing comment easier to understand.
For example, you've changed this so now it ends like so:
#endif // defined(__INTEL_PREVIEW_BREAKING_CHANGES)
Which would lead the casual reader to think ('oh, this block above is for the preview") which is wrong. The block immediately above is the ELSE clause of the preview.
I know it seems pedantic, but especially with this preview stuff where we have long blocks of semi-repeating code, let's try to avoid #else in long blocks
There was a problem hiding this comment.
Personally, I disagree with that, but it is not a hill I want to die on. We could simply change the comment on the #endif to make it clearer and the #else, I would argue, makes the association clearer between the two blocks. Either way, I've split them up.
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
steffenlarsen
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
This commit does a partial revert of intel#12396 as it is unclear from the specification how these operators should behave for swizzles. Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
This commit changes operators for sycl::vec to be defined like they are in the SYCL specification, i.e. friend functions instead of members.