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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to