[PATCH] D78979: OpenCL: Include builtin header by default

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D78979#2015214 <https://reviews.llvm.org/D78979#2015214>, @arsenm wrote:

> In D78979#2010739 <https://reviews.llvm.org/D78979#2010739>, @Anastasia wrote:
>
> > In D78979#2010527 <https://reviews.llvm.org/D78979#2010527>, @svenvh wrote:
> >
> > > In D78979#2007643 <https://reviews.llvm.org/D78979#2007643>, @Anastasia wrote:
> > >
> > > > > Would it not become confusing since the builtins are going to be included by default? Should we rename the flag at least? Also ideally it should be documented in https://clang.llvm.org/docs/UsersManual.html#opencl-header
> > > >
> > > > ah I guess if we leave it under `-cc1 ` we will have the command line interface as follows:
> > > >
> > > > - Driver (without `-cc1`) adds OpenCL header by default that can be overridden by the flag added in this patch.
> > > > - Frontend (with `-cc1`) doesn't add the header by default but there are two flags `-fdeclare-opencl-builtins` or `-finclude-default-header` that allow to include the header.
> > >
> > >
> > > The name of `-fdeclare-opencl-builtins` becomes a bit non-intuitive, a more intuitive name would be something like `-fopencl-tablegen-builtins` perhaps or `-fopencl-fast-builtins` if we don't want to mention TableGen.  But since it is still an experimental option, I have no objections against postponing the renaming until the option has reached maturity.
> >
> >
> > I vote for `-fopencl-fast-builtins`.
>
>
> The main problem I'm trying to solve is that users shouldn't need to concern themselves with including any header, much less a choice between two internal implementation details. Switching the two internal header implementations seems OK to leave as a cc1 flag. The main question I think is what the spelling of how to disable whatever default header was chosen, and what the internal flags to switch the implementation look like.
>
> So what's the agreement on spelling? Use -nostdinc/-nostdlib? Have 2 different cc1 flags for the header implementation switch? Surface/invert -fno-include-default-header?


I like the idea of reusing -nostdinc/-nostdlib, although we don't link to libs by default so I guess -nostdlib is useless? Then do we want to keep `-finclude-default-header` to be used for the frontend? As for the TableGen header I guess we can leave it under frontend only flag for now but I would suggest to rename to something more distinct i.e. `-fopencl-fast-builtins`?


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

https://reviews.llvm.org/D78979