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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
