azabaznov added inline comments.
================
Comment at: clang/lib/Sema/Sema.cpp:364
for (auto &I : Atomic64BitTypes)
setOpenCLExtensionForType(I,
"cl_khr_int64_base_atomics cl_khr_int64_extended_atomics");
----------------
Anastasia wrote:
> I think this should be lifted into the above if statement? Although since we
> are adding the types conditionally we could even drop this code completely
> and let the developers just use the types as they are already added and made
> available by the frontend.
> I think this should be lifted into the above if statement?
No, this is needed to be here as `atomic_double` added above.
> Although since we are adding the types conditionally we could even drop this
> code completely and let the developers just use the types as they are already
> added and made available by the frontend.
I'm now worried about diagnostics with that change:
```
tmp.cl:2:17: error: expected ';' after expression
atomic_ulong a;
^
;
tmp.cl:2:5: error: use of undeclared identifier 'atomic_ulong'
atomic_ulong a;
^
tmp.cl:2:18: error: use of undeclared identifier 'a'
atomic_ulong a;
^
tmp.cl:3:5: error: unknown type name 'atomic_uintptr_t'; did you mean
'atomic_uint'?
atomic_uintptr_t u;
^~~~~~~~~~~~~~~~
atomic_uint
note: 'atomic_uint' declared here
4 errors generated.
```
Is this expected behaviour when `cl_khr_int64_base_atomics` and
'cl_khr_int64_extended_atomics' not supported? I think we need o be more
careful with reserved identifiers. However, I didn't find explicit wording
about it, but I'm pretty sure that all of these atomic types identifiers should
be reserved.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97058/new/
https://reviews.llvm.org/D97058
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
