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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
