[PATCH] D95523: [OpenCL] Add cl_khr_subgroup_ballot to TableGen BIFs
Anastasia Stulova via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 2 04:46:33 PST 2021
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.
LGTM! Thanks!
================
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__)
----------------
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...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95523/new/
https://reviews.llvm.org/D95523
More information about the cfe-commits
mailing list