svenvh added inline comments.
================
Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:35
+// Enable extensions that are enabled in opencl-c-base.h.
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
+#define cl_khr_subgroup_ballot 1
----------------
Anastasia wrote:
> Not related to this patch - I think we should have included `opencl-c-base.h`
> by default when passing `-fdeclare-opencl-builtins` or do you see any issues
> with that?
>
>
That's something we probably want to do in the future indeed.
================
Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:141
+kernel void extended_subgroup(global uint4 *out) {
+ out[0] = get_sub_group_eq_mask();
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0 && !defined(__OPENCL_CPP_VERSION__)
----------------
Anastasia wrote:
> I appreciate we haven't addressed the standard header testing holistically
> yet and this functionality was added in `opencl-c.h` without testing the
> functions, but would it be better to test each function at least here?
>
> Even though the final goal should be testing all overloads, this is outside
> of the scope of this work that is closing the gap between `opencl-c.h` and
> this experimental function declaration mechanism.
> but would it be better to test each function at least here?
I think that defeats the purpose of this test. This test is meant to test the
basic functionality of `-fdeclare-opencl-builtins`. It is not a completeness
test.
A completeness test shouldn't be tied to `-fdeclare-opencl-builtins` alone;
such a test should also be run against `opencl-c.h`, so this would be the wrong
test to turn into a completeness test in my opinion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95523/new/
https://reviews.llvm.org/D95523
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
