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

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

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?


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

https://reviews.llvm.org/D78979