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