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:
> > > > 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?
> > > I think SYCL model is quite similar to OpenMP model. One significant
> > > difference might be that to implement standard SYCL functionality we
> > > don't need any modifications for the host compiler. AFAIK, OpenMP
> > > compiler has to support OpenMP pragmas.
> > > We have a few attributes for Intel FPGA devices, which we can't
> > > pre-process with `__SYCL_DEVICE_ONLY__` macro and we have added
> > > "SYCL-host" mode to suppress compiler warnings about attributes ignored
> > > on the host. I think there might be other options how this can be
> > > achieved w/o adding new compilation mode and use regular C++ front-end as
> > > SYCL host compiler.
> > > I think there is a difference between SYCL and SYCL-device modes, but
> > > it's mostly changes the compilation workflow in the driver, but not in
> > > the front-end. In SYCL-device mode, driver invokes only one front-end
> > > instance to generate offload code. In SYCL mode, driver invokes multiple
> > > front-end instances: one in SYCL-device mode and one in regular C++ mode
> > > (to be accurate we use SYCL-host mode, but as I mentioned above I don't
> > > think it really needed).
> > > I hope it makes it clear now. Let me know if you have any other questions.
> > >
> > > Do I understand it correctly that OpenMP option enables OpenMP mode,
> > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and
> > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar to
> > > SYCL-device mode?
> > > If so, can we assume that OpenMPIsDevice implies that OpenMP option is
> > > also set (implicitly)?
> > > Do I understand it correctly that OpenMP option enables OpenMP mode,
> > > which is equivalent of "SYCL-host" mode and enabling both OpenMP and
> > > OpenMPIsDevice is required for enabling OpenMP mode, which is similar to
> > > SYCL-device mode?
> >
> > Well, for driver you need to pass `-fopenmp
> > -fopenmp-target=<list_of_targets>` to enable the compilation with
> > offloading support. In the frontend the host part is compiled with
> > `-fopenmp` only (+ aux-triple, probably), for devices - `-fopenmp
> > -fopenmp-is-device`. Without `-fopenmp` `-fopenmp-is-device` is just
> > ignored.
> What is the reason to require the driver to pass both options for the
> devices? It sounds like `-fopenmp-is-device` should be enough to
> differentiate from the host part (`-fopenmp` only). Right?
We treat a little bit differently. `-fopenmp` turns support for OpenMP, while
`-fopenmp-is-device` turns processing for device compilation.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72857/new/
https://reviews.llvm.org/D72857
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
