[PATCH] D124220: [OpenMP] Add options to only compile the host or device when offloading

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:2527
   HelpText<"Use the static host OpenMP runtime while linking.">;
+def offload_device_only : Flag<["-"], "offload-device-only">,
+  HelpText<"Only compile for the offloading device.">;
----------------
jhuber6 wrote:
> tra wrote:
> > We should be using "--" for the new options.
> What's the reason for preferring `--` over `-`? Is it just because `-o` can bind to output?
Convention, I guess. Legacy (e.g. `-nostdlib` )  and single-letter options (e.g. `-o` `-m`, `-f` ) use single dash. Long options typically use double-dash.


================
Comment at: clang/lib/Driver/Driver.cpp:4215
                                        Action *HostAction) const {
-  if (!isa<CompileJobAction>(HostAction))
+  if (Args.hasArg(options::OPT_offload_host_only))
     return HostAction;
----------------
jhuber6 wrote:
> tra wrote:
> > This will not always be correct. E.g. `--offload-host-only --offload-host-device` would be true here, but we would still want to compile for both and device.
> > 
> > Is there a reason we can no longer rely on `HostAction`?
> Guess I should just use the last argument. Host action is used below, can probably merge these if statements.
I guess the general idea is to avoid the ambiguity about what controls the behavior of the function. Is that the `Args`, or the `HostAction`?
Ideally I'd prefer to parse command line options once, save results somewhere we could use them and then use those flargs to control the behavior, regardless of which options were used to specify it. E.g. CUDA/HIPActionBuilder classes have these member fields:
```
  bool CompileHostOnly = false;
  bool CompileDeviceOnly = false;

...
      Arg *PartialCompilationArg = Args.getLastArg(
          options::OPT_cuda_host_only, options::OPT_cuda_device_only,
          options::OPT_cuda_compile_host_device);
      CompileHostOnly = PartialCompilationArg &&
                        PartialCompilationArg->getOption().matches(
                            options::OPT_cuda_host_only);
      CompileDeviceOnly = PartialCompilationArg &&
                          PartialCompilationArg->getOption().matches(
                              options::OPT_cuda_device_only);
...
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124220