svenvh added inline comments.
================
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:
> svenvh wrote:
> > 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.
> Ok, this test does test functionality selectively but I wouldn't be able to
> tell how one selects what to test. Perhaps a comment would help for the
> future development...
There are no formal selection criteria here I think. The test aims to cover
the different functional aspects of `-fdeclare-opencl-builtins`. The new
functional aspect for `-fdeclare-opencl-builtins` in this patch is an extension
that is not in `OpenCLExtensions.def`, so it makes sense to add such a case to
the test.
I will add a comment to the top of the test file.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95523/new/
https://reviews.llvm.org/D95523
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
