[PATCH] D137652: Remove mandatory define of optional features macros for OpenCL C 3.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D137652#3957233 <https://reviews.llvm.org/D137652#3957233>, @FMarno wrote:

> @Anastasia I feel like I am following the guidance you quoted here. The defines I've deleted in `opencl-c-base.h` are blocking the possibility of `-cl-ext` working and would also get in the way of undefine preprocessor symbols working. 
> If there is a problem with the additions to `OpenCLExtensions.def` then we'll need to rework how `InitializeOpenCLFeatureTestMacros` in `initPreprocessor.cpp` works.

Let me clarify this topic a bit better -  the guidelines are about whether a feature/extension needs to be added into the frontend based on whether the frontend needs to know about the feature and do something different e.g. for example it can parse the code differently and etc. However you are adding the features just in order for them to appear in the predefined macros. The approach you choose is a workaround because ideally it should only be used when the fronted needs to query the feature settings to do something useful based on those but you don't ever need them in the frontend as far as I understand as you only need to add the macros. Since clang includes default header files it is very straightforward to add the macros in the headers by simply defining them there. No frontend changes needed for that. Now the only problem is that the headers don't provide defining the macros conditionally yet because nobody has added this functionality yet.

One idea that we discussed in the issue I quoted is to extend `-cl-ext` such that it would be able to define or undefine those macros, or alternatively feature/extensions macro defines in the headers could be guarded by the corresponding `__undef_<feature/extension name>` macros that can be added from the command line during source compilation by either user or a compiler driver. It is also possible to use a hybrid approach i.e. `-cl-ext` would add `__undef_<feature/extension name>` macros that guard the feature/extension macros in the header. This mechanism of adding header only feature/extension macros is not fully working yet as it is currently only sets macros per target. I appreciate that a more fine grained control is desirable in some situation as in your case. However this doesn't mean that we should continue adding more thing into the frontend where it is  not needed. We should work toward more scalable solution instead that provides such control in the internal headers. Does this make the direction clear? I would recommend to check the discussions in the issue (#55674) and linked review as it provides more background.

@svenvh I remember that we have also discussed the addition of a vendor specific header where such feature/extension macro definition can be added to avoid the macro pollution but I feel this is somewhat orthogonal i.e. the fine grained control of macro defines is still needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137652