[PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: include/clang/Sema/Sema.h:8081
+  /// \brief Set current OpenCL extensions for a type which can only be used
+  /// when these OpenCL extensions are enabled. If current OpenCL Extsion is
+  /// empty, do nothing.
----------------
Extsion -> extension


================
Comment at: include/clang/Sema/Sema.h:8120
 
+  /// Checks if a type or declaration is disabled due to the owning extension
+  /// is disabled, and emits diagnostic messages if it is disabled.
----------------
or declaration -> or a declaration


================
Comment at: include/clang/Sema/Sema.h:8121
+  /// Checks if a type or declaration is disabled due to the owning extension
+  /// is disabled, and emits diagnostic messages if it is disabled.
+  /// \param D type or declaration to be checked.
----------------
is disabled -> being disabled


================
Comment at: lib/Serialization/ASTReader.cpp:3167
     case OPENCL_EXTENSIONS:
-      // Later tables overwrite earlier ones.
-      OpenCLExtensions.swap(Record);
----------------
Btw, OpenCLTypeExtMap and OpenCLTypeDeclMap don't have to be serialized?


================
Comment at: test/Parser/opencl-atomics-cl20.cl:51
 // expected-error at -28 {{use of type 'atomic_double' (aka '_Atomic(double)') requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re at -27 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re at -28 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re at -28 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re at -29 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re at -29 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re at -30 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re at -30 {{use of type 'atomic_ptrdiff_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re at -31 {{use of type 'atomic_ptrdiff_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be enabled}}
+#if __LP64__
+// expected-error-re at -28 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
----------------
yaxunl wrote:
> Anastasia wrote:
> > Why this change?
> atomic_intptr_t etc. requires cl_khr_int64_extended_atomics only on 64 bit platforms.
> 
> This is a bug which was fixed by this patch.
The spec says:
"If the device address space is 64-bits, the data types atomic_intptr_t, atomic_uintptr_t,
atomic_size_t and atomic_ptrdiff_t are supported if the cl_khr_int64_base_atomics and
cl_khr_int64_extended_atomics extensions are supported."

This seems to be the same as long and double?


https://reviews.llvm.org/D21698