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