[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

Wed Mar 3 09:28:23 PST 2021

Anastasia added a comment.

In D97058#2600160 <https://reviews.llvm.org/D97058#2600160>, @azabaznov wrote:

> Check 'isEnabled' is now private: it is used only for non-core or non-optional core features;
> creation of implicit type definitions is guarder with extension support check; minor refactoring

Ok, I think we could remove further the differences between extensions and features wrt pragmas as suggested in the comments but it is also ok if we do separately. This is definitely a step closer to the end goal of simplifying and unifying the handling of extensions and features and removing the confusion about the pragmas.

================
Comment at: clang/lib/Sema/Sema.cpp:364
       for (auto &I : Atomic64BitTypes)
         setOpenCLExtensionForType(I,
             "cl_khr_int64_base_atomics cl_khr_int64_extended_atomics");
----------------
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.

================
Comment at: clang/lib/Sema/Sema.cpp:376
+    addImplicitTypedef(#ExtType, Context.Id##Ty);                              \
+    setOpenCLExtensionForType(Context.Id##Ty, #Ext);                           \
+  }
----------------
Since we are guarding by the feature/extension support we could even remove the need for pragma in order to use the types?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97058/new/

https://reviews.llvm.org/D97058