[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {
----------------
bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > This option also must be controlled by `-fsycl`:
> > > > ```
> > > > Opts.SYCLIsDevice =  Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
> > > > 
> > > > ```
> > > Does it really has to? This logic is already present in the driver and it makes front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
> > > Can `-fsycl-is-device` imply `-fsycl`?
> > > Looking how CUDA/OpenMP options are handled, not all of them are processed using this pattern.
> > In general, this is how we handle it in OpenMP. Cuda works differently, because it has its own kind of files (.cu) and Cuda is triggered by the language switch (-x cu). Seems to me, you're using something close to OpenMP model, no? Or do you want to define your own language kind just like Cuda?
> I applied you suggest, although I don't fully understand the need of using two options instead of two. I would prefer having following code:
> ```
> Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
> Opts.SYCL = Args.hasArg(options::OPT_fsycl) || Opts.SYCLIsDevice; // -fsycl-is-device enable SYCL mode as well
> ```
I'm not quite familiar with SYCL model, maybe this the right approach. You'd better try to provide more details. Are there any differences between just SYCL and SYCL-device compilation modes? How do you see the compilation sequence in general? At first you're going to compile the host version of the code, then the device? OR something different?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857