-
Notifications
You must be signed in to change notification settings - Fork 736
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] Make swizzle mutating operators const friends #13012
[SYCL] Make swizzle mutating operators const friends #13012
Conversation
In intel#12682 the mutating operators for swizzles (+=, -=, ..., ++, --) were reverted to be members rather than friends. Since swizzles mutate the underlying vec rather than themselves these operators should take and return constant references instead, which this commit implements. Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Swizzles should not expose mutating functions when the underlying vector is const. This commit SFINAEs these out. This is built on top of intel#13012. Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
*(ResultIt++) = (VecVal.swizzle<1>() OP## = 2)[0] == ExpectedRes && \ | ||
VecVal[1] == ExpectedRes; \ | ||
} | ||
TestCase(+); |
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 wonder if we can do TestCase([](auto x, auto y) { return x + y; })
to avoid the macro.
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 sure that would help here. Note that in this case the line will resolve to:
{
sycl::vec<int, 4> VecVal{1, 2, 3, 4};
int ExpectedRes = VecVal[1] + 2;
ResultsAcc[I++] = (VecVal.swizzle<1>() += 2)[0] == ExpectedRes &&
VecVal[1] == ExpectedRes;
}
The important check here is really the +=
as it is the mutating function, but I can see if the TestCase
macro is a little hard to read, due to the space the formatter adds in OP##=
.
I could change it to be TestCase(+, +=)
instead, but I'm not sure if there's a way to make it take a lambda function without it being way more verbose.
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
KhronosGroup/SYCL-Docs#514 has been merged. This is ready for review! |
Sorry, grabbed the wrong tag. @intel/llvm-reviewers-runtime - Friendly ping. |
I believe the change is rather isolated and can be merged without re-test. |
In intel#12682 the mutating operators for swizzles (+=, -=, ..., ++, --) were reverted to be members rather than friends. Since swizzles mutate the underlying vec rather than themselves these operators should take and return constant references instead, which this commit implements. --------- Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
In #12682 the mutating operators for swizzles (+=, -=, ..., ++, --) were reverted to be members rather than friends. Since swizzles mutate the underlying vec rather than themselves these operators should take and return constant references instead, which this commit implements.