[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.


================
Comment at: clang/include/clang/Basic/OpenCLExtensions.def:71
+OPENCL_EXTENSION(cl_khr_int64_extended_atomics, true, 100)
+OPENCL_COREFEATURE(cl_khr_3d_image_writes, true, 100, OCL_C_20)
 
----------------
Anastasia wrote:
> azabaznov wrote:
> > I think core and optional core features do not require pragma too. So for example 3d image writes or fp64 do not require pragma in OpenCL C 2.0. Isn't that so?
> Well to be honest nothing seems to need pragma but we have to accept it for the backward compatibility. :)
> 
> In case of the core features if pragma would have been useful we would have to still accept it for the backward compatibility even if the feature became core.
I'm just wondering if this new field needed in the file to maintain backward compatibility. Maybe we can highlight OpenCL C 3.0 features with some other way? Is it seems that check for name starting with "__opencl_c" is a bad idea?


================
Comment at: clang/lib/Parse/ParsePragma.cpp:785
+      // Therefore, it should never be added by default.
+      Opt.acceptsPragma(Name);
     }
----------------
Anastasia wrote:
> svenvh wrote:
> > I fail to understand why this is needed, so perhaps this needs a bit more explanation.  Extensions that should continue to support pragmas already have their `WithPragma` field set to `true` via `OpenCLExtensions.def`.  Why do we need to dynamically modify the field?
> It is a bit twisty here to be honest. Because we have also introduced the pragma `begin` and `end` that would add pragma `enable`/`disable` by default. So any extension added dynamically using `begin`/`end` would have to accept the pragma `enable`/`disable`. 
> 
> https://clang.llvm.org/docs/UsersManual.html#opencl-extensions
> 
> But in the subsequent patches, I hope to remove this because I just don't see where it is useful but it is very confusing.
Is it ok not to track this situation here:

```
#pragma OPENCL EXTENSION __opencl_c_feature : begin
#pragma OPENCL EXTENSION __opencl_c_feature: end
```

This is some of a corner case, but still...


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

https://reviews.llvm.org/D97052