svenvh added inline comments.
================
Comment at: clang/lib/Sema/OpenCLBuiltins.td:932
+let MinVersion = CL20 in {
+ def : Builtin<"get_fence", [MemFenceFlags, PointerType<Void, GenericAS>]>;
+ def : Builtin<"get_fence", [MemFenceFlags, PointerType<ConstType<Void>,
GenericAS>]>;
----------------
Anastasia wrote:
> Btw, I am guessing you are matching the behavior of `opencl-c.h`, but however
> it doesn't seem entirely conformant.
>
> `cl_mem_fence_flags get_fence(gentype *ptr)`
>
> So I guess we should have implemented these as other functions from `Table
> 22. Built-in Address Space Qualifier Functions` in clang? But since the void
> pointer is only used as a parameter and not return value I doubt this is
> customer visible. Should we add a comment or something?
>
>
>
Yes, I'm matching `opencl-c.h` here, as the "gentype argument to indicate any
of the built-in data types supported by OpenCL C or a user defined type. Since
this cannot be captured" can't be expressed literally. I can add the following
comment if that makes sense?
```// The OpenCL 3.0 specification defines these with a "gentype" argument
indicating any builtin
// type or user-defined type, which cannot be represented currently. Hence we
slightly diverge
// from this by providing only the following two overloads with a void pointer.
```
================
Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:185
+#if !defined(NO_HEADER) && (defined(__OPENCL_CPP_VERSION__) ||
__OPENCL_C_VERSION__ >= 200)
+ sub_group_barrier(CLK_GLOBAL_MEM_FENCE, memory_scope_device);
+#endif
----------------
Anastasia wrote:
> Should `work_group_barrier` and fences also be added to the testing?
MemFenceFlags are already tested using `barrier()` on line 23, so not strictly
needed. I added this test to verify the combination of MemFenceFlags and
extensions. Do you think we're missing any tablegen functionality coverage
that would require adding more builtins to the test (also taking into account
the comment on lines 10-13)?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96860/new/
https://reviews.llvm.org/D96860
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
