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

Tue Mar 2 03:37:35 PST 2021

Anastasia added a comment.

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

>> FYI, I would even be ok if we remove the need for enabling non-core
>
> AFAIU this can be done not for every extension to maintain backward compatibility.

This should not result in any backward compatibility issues, as the old kernels using `pragma enable` will still compile successfully.

> Also, I imagine that implicit type definition is not  needed if no pragma required.

They need to be added but conditionally on language version or extension/feature support. If there is no support they should not be added as identifiers by the compiler and therefore can be reused by the developers in the application code.

================
Comment at: clang/lib/Sema/Sema.cpp:360

     setOpenCLExtensionForType(Context.DoubleTy, "cl_khr_fp64");

----------------
azabaznov wrote:
> Anastasia wrote:
> > I think the same should apply to `double` not only `atomic_double`?
> I think `double` is created in a place where it's not possible to guard it with OpenCL extensions support. I will double-check.
That's right, `double` is a reserved identifier so we should always accept it. However, we should allow its use as a legal type without the pragma if `fp64` is supported. We could remove this statement here and add a separate check for it in `Sema`, but we could also modify this mechanism to check for the extensions/features being supported instead of being enabled.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97058