bader marked 3 inline comments as done.
bader 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) {
----------------
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
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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

Reply via email to