Anastasia created this revision.
Anastasia added reviewers: svenvh, azabaznov.
Herald added subscribers: ebevhan, jfb, yaxunl.
Anastasia requested review of this revision.
Clang's implementation of the extension pragma is broken because it doesn't
respect the following requirement from OpenCL Extension spec s1.2:
https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_Ext.html#extensions-overview
> **disable** Behave (including issuing errors and warnings) as if the
> extension extension_name is not part of the language definition.
This means that extension functionality should not be exposed by default and if
extension identifiers are not reserved they should not be recognized by the
compiler. This is for example the case for some of the atomic types (i.e. 64
bit types) from extensions.
As fixing the implementation of extension pragma is not feasible and it will
affect backward compatibility and require significant rework in the parser this
patch simplifies the implementation of pragma by making the behavior as
consistent and intuitive as possible.
This change removes the requirement on pragma when atomic types from the
extensions are supported because the behavior is not conformant. With this
change, the developers can use atomic types from the extensions if they are
supported without enabling the pragma because disabling the pragma doesn't do
anything useful but only prevents the use of already available identifiers for
types and issues extra diagnostics. This makes semantics consistent with the
atomic functions that are also available when the extension is supported
without any need for the pragma.
This patch does not break backward compatibility since the extension pragma is
still supported and it makes the behavior of the compiler less strict by
accepting code without extra pragma statements.
Note that there is an ongoing thread to alter the behavior in the spec
https://github.com/KhronosGroup/OpenCL-Docs/issues/82
however this change is orthogonal as it simplifies the current implementation
that is not conformant.
Any new functionality can be added later if the behavior in the spec is fixed.
https://reviews.llvm.org/D100976
Files:
clang/lib/Sema/Sema.cpp
clang/test/Parser/opencl-atomics-cl20.cl
Index: clang/test/Parser/opencl-atomics-cl20.cl
===================================================================
--- clang/test/Parser/opencl-atomics-cl20.cl
+++ clang/test/Parser/opencl-atomics-cl20.cl
@@ -1,67 +1,76 @@
// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20
-// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20 -DEXT -Wpedantic-core-features
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -cl-ext=-cl_khr_int64_base_atomics
+// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CLC++
+// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -cl-ext=-cl_khr_int64_base_atomics
-#ifdef EXT
-#pragma OPENCL EXTENSION cl_khr_int64_base_atomics:enable
-#pragma OPENCL EXTENSION cl_khr_int64_extended_atomics:enable
-#pragma OPENCL EXTENSION cl_khr_fp64:enable
-#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
-// expected-warning@-2{{OpenCL extension 'cl_khr_fp64' is core feature or supported optional core feature - ignoring}}
-#endif
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#define LANG_VER_OK
#endif
void atomic_types_test() {
-// OpenCL v2.0 s6.13.11.6 defines supported atomic types.
+ // OpenCL v2.0 s6.13.11.6 defines supported atomic types.
+
+ // Non-optional types
atomic_int i;
atomic_uint ui;
+ atomic_float f;
+ atomic_flag fl;
+#if !defined(LANG_VER_OK)
+// expected-error@-5 {{use of undeclared identifier 'atomic_int'}}
+// expected-error@-5 {{use of undeclared identifier 'atomic_uint'}}
+// expected-error@-5 {{use of undeclared identifier 'atomic_float'}}
+// expected-error@-5 {{use of undeclared identifier 'atomic_flag'}}
+#endif
+
+ // Optional types
atomic_long l;
atomic_ulong ul;
- atomic_float f;
atomic_double d;
- atomic_flag fl;
+ atomic_size_t s;
atomic_intptr_t ip;
atomic_uintptr_t uip;
- atomic_size_t s;
atomic_ptrdiff_t pd;
-// OpenCL v2.0 s6.13.11.8, _Atomic type specifier and _Atomic type qualifier
-// are not supported by OpenCL.
- _Atomic int i; // expected-error {{use of undeclared identifier '_Atomic'}}
-}
-#ifndef CL20
-// expected-error@-16 {{use of undeclared identifier 'atomic_int'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_uint'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_long'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_ulong'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_float'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_double'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_flag'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_intptr_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_uintptr_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_size_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_ptrdiff_t'}}
-#elif !EXT
-// expected-error@-26 {{use of type 'atomic_long' (aka '_Atomic(long)') requires cl_khr_int64_base_atomics support}}
-// expected-error@-27 {{use of type 'atomic_long' (aka '_Atomic(long)') requires cl_khr_int64_extended_atomics support}}
-// expected-error@-27 {{use of type 'atomic_ulong' (aka '_Atomic(unsigned long)') requires cl_khr_int64_base_atomics support}}
-// expected-error@-28 {{use of type 'atomic_ulong' (aka '_Atomic(unsigned long)') requires cl_khr_int64_extended_atomics support}}
-// expected-error@-27 {{use of type 'atomic_double' (aka '_Atomic(double)') requires cl_khr_int64_base_atomics support}}
-// expected-error@-28 {{use of type 'atomic_double' (aka '_Atomic(double)') requires cl_khr_int64_extended_atomics support}}
-#if __LP64__
-// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-29 {{use of type 'atomic_intptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re@-29 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-30 {{use of type 'atomic_uintptr_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re@-30 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-31 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
-// expected-error-re@-31 {{use of type 'atomic_ptrdiff_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_base_atomics support}}
-// expected-error-re@-32 {{use of type 'atomic_ptrdiff_t' (aka '_Atomic({{.+}})') requires cl_khr_int64_extended_atomics support}}
+#if !defined(LANG_VER_OK) || !defined(cl_khr_int64_base_atomics)
+// expected-error@-8 {{use of undeclared identifier 'atomic_long'}}
+// expected-error@-8 {{use of undeclared identifier 'atomic_ulong'}}
+// expected-error@-8 {{use of undeclared identifier 'atomic_double'}}
+#if defined(LANG_VER_OK)
+// expected-error@-12 {{expected ';' after expression}}
+// expected-error@-13 {{use of undeclared identifier 'l'}}
+// expected-error@-13 {{expected ';' after expression}}
+// expected-error@-14 {{use of undeclared identifier 'ul'}}
+#endif
+#if !defined(LANG_VER_OK) || defined(__SPIR64__)
+// expected-error@-15 {{use of undeclared identifier 'atomic_size_t'}}
+// expected-error@-13 {{use of undeclared identifier 'atomic_ptrdiff_t'}}
+#if !defined(LANG_VER_OK)
+// expected-error@-17 {{use of undeclared identifier 'atomic_intptr_t'}}
+// expected-error@-17 {{use of undeclared identifier 'atomic_uintptr_t'}}
+#else
+// expected-error@-21 {{expected ';' after expression}}
+// expected-error@-22 {{use of undeclared identifier 's'}}
+// expected-error@-22 {{unknown type name 'atomic_intptr_t'; did you mean 'atomic_int'?}}
+// expected-note@* {{'atomic_int' declared here}}
+// expected-error@-23 {{unknown type name 'atomic_uintptr_t'; did you mean 'atomic_uint'?}}
+// expected-note@* {{'atomic_uint' declared here}}
#endif
#endif
+#endif
+
+ // OpenCL v2.0 s6.13.11.8, _Atomic type specifier and _Atomic type qualifier
+ // are not supported by OpenCL.
+ _Atomic int i;
+#ifdef __OPENCL_C_VERSION__
+// expected-error@-2 {{use of undeclared identifier '_Atomic'}}
+#else
+ // expected-error@-4 {{unknown type name '_Atomic'}}
+#endif
+}
-#ifdef CL20
-void foo(atomic_int * ptr) {}
+#if defined(LANG_VER_OK)
+int atomic_uint; // expected-error{{redefinition of 'atomic_uint' as different kind of symbol}}
+void foo(atomic_int *ptr) {}
void atomic_ops_test() {
atomic_int i;
foo(&i);
@@ -71,4 +80,6 @@
i += 1; // expected-error {{invalid operands to binary expression ('__private atomic_int' (aka '__private _Atomic(int)') and 'int')}}
i = i + i; // expected-error {{invalid operands to binary expression ('__private atomic_int' (aka '__private _Atomic(int)') and '__private atomic_int')}}
}
+#else
+__constant int atomic_uint = 1;
#endif
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -313,8 +313,6 @@
// OpenCLC v2.0, s6.13.11.6 requires that atomic_flag is implemented as
// 32-bit integer and OpenCLC v2.0, s6.1.1 int is always 32-bit wide.
addImplicitTypedef("atomic_flag", Context.getAtomicType(Context.IntTy));
- auto AtomicSizeT = Context.getAtomicType(Context.getSizeType());
- addImplicitTypedef("atomic_size_t", AtomicSizeT);
// OpenCL v2.0 s6.13.11.6:
// - The atomic_long and atomic_ulong types are supported if the
@@ -327,6 +325,23 @@
// 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.
+
+ auto AddPointerSizeDependentTypes = [&]() {
+ auto AtomicSizeT = Context.getAtomicType(Context.getSizeType());
+ auto AtomicIntPtrT = Context.getAtomicType(Context.getIntPtrType());
+ auto AtomicUIntPtrT = Context.getAtomicType(Context.getUIntPtrType());
+ auto AtomicPtrDiffT =
+ Context.getAtomicType(Context.getPointerDiffType());
+ addImplicitTypedef("atomic_size_t", AtomicSizeT);
+ addImplicitTypedef("atomic_intptr_t", AtomicIntPtrT);
+ addImplicitTypedef("atomic_uintptr_t", AtomicUIntPtrT);
+ addImplicitTypedef("atomic_ptrdiff_t", AtomicPtrDiffT);
+ };
+
+ if (Context.getTypeSize(Context.getSizeType()) == 32) {
+ AddPointerSizeDependentTypes();
+ }
+
std::vector<QualType> Atomic64BitTypes;
if (getOpenCLOptions().isSupported("cl_khr_int64_base_atomics",
getLangOpts()) &&
@@ -335,35 +350,17 @@
if (getOpenCLOptions().isSupported("cl_khr_fp64", getLangOpts())) {
auto AtomicDoubleT = Context.getAtomicType(Context.DoubleTy);
addImplicitTypedef("atomic_double", AtomicDoubleT);
- setOpenCLExtensionForType(AtomicDoubleT, "cl_khr_fp64");
Atomic64BitTypes.push_back(AtomicDoubleT);
}
auto AtomicLongT = Context.getAtomicType(Context.LongTy);
auto AtomicULongT = Context.getAtomicType(Context.UnsignedLongTy);
- auto AtomicIntPtrT = Context.getAtomicType(Context.getIntPtrType());
- auto AtomicUIntPtrT = Context.getAtomicType(Context.getUIntPtrType());
- auto AtomicPtrDiffT =
- Context.getAtomicType(Context.getPointerDiffType());
-
addImplicitTypedef("atomic_long", AtomicLongT);
addImplicitTypedef("atomic_ulong", AtomicULongT);
- addImplicitTypedef("atomic_intptr_t", AtomicIntPtrT);
- addImplicitTypedef("atomic_uintptr_t", AtomicUIntPtrT);
- addImplicitTypedef("atomic_ptrdiff_t", AtomicPtrDiffT);
- Atomic64BitTypes.push_back(AtomicLongT);
- Atomic64BitTypes.push_back(AtomicULongT);
- if (Context.getTypeSize(AtomicSizeT) == 64) {
- Atomic64BitTypes.push_back(AtomicSizeT);
- Atomic64BitTypes.push_back(AtomicIntPtrT);
- Atomic64BitTypes.push_back(AtomicUIntPtrT);
- Atomic64BitTypes.push_back(AtomicPtrDiffT);
+ if (Context.getTypeSize(Context.getSizeType()) == 64) {
+ AddPointerSizeDependentTypes();
}
}
-
- for (auto &I : Atomic64BitTypes)
- setOpenCLExtensionForType(I,
- "cl_khr_int64_base_atomics cl_khr_int64_extended_atomics");
}
setOpenCLExtensionForType(Context.DoubleTy, "cl_khr_fp64");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
