tra added inline comments.
================ Comment at: clang/include/clang/Driver/Options.td:2535-2538 +def foffload_device_only : Flag<["-"], "foffload-device-only">, + HelpText<"Only compile for the offloading device.">; +def foffload_host_only : Flag<["-"], "foffload-host-only">, + HelpText<"Only compile for the offloading host.">; ---------------- jhuber6 wrote: > tra wrote: > > jhuber6 wrote: > > > tra wrote: > > > > We should probably alias `--cuda-host-only`/`--cuda-device-only` to > > > > these. > > > Unfortunately I can't make these a strict alias for > > > `-foffload-host-only`/`-foffload-device-only` without breaking their > > > current usage. I could just check them along with these new versions, but > > > that makes it a little cluttered. > > Could you give me an example why we can use just one set of options to > > control host-only and device-only compilation? > > > > Is that because we also have `--cuda-compile-host-device`? > > Or is it because `-f` options have `-fno-` variants to override them, while > > `--cuda-host/device-only` don't, so we can't just mark them as an alias in > > Options.td? > > To think of it, those are just facets of the same issue -- difference in > > the options syntax. > > I do not see semantic differences between the existing options and > > -foffload-*-only. > > > > E.g. I believe you could've used --cuda-host-only and --cuda-device-only > > instead of -foffload-*-only without the loss of functionality of your patch. > > > I could have used those options, but this applies in general to the new > driver, so it wouldn't be intuitive if we used `--cuda-device-only` when > offloading to x86_64 with OpenMP for example. The problem is that the > `--cuda-device-only` is already used in the current offloading driver so if I > just make it an alias to this option it won't work. I was hoping to avoid > touching the old driver, but I guess I could go back and replace all uses of > `--cuda-host/device-only` with these new options. Also I did forget to add > the `fno-` variants for these and use them correctly. I'm not sure if there's > an explicit reason to, but everything else in the OpenMP world uses `-f` > prefixes so I'm mostly just sticking with that. > it wouldn't be intuitive if we used --cuda-device-only when offloading to > x86_64 with OpenMP On the other hand, adding another set of options replicating the functionality creates more opportunities for conflicting usage. E.g. what should we do if user specifies `--cuda-device-only` and `-foffload-device-only` ? We then need to diagnose that in a sensible way, at the very least. In general, we have been transitioning some options that used to be CUDA-specific into more general variants. Usually `--cuda*` -> `--offload*` (--cuda-gpu-arch->--offload-arch) or `--gpu*`. (-fcuda-rdc -> -fgpu-rdc) Picking the offloading side should follow the same pattern. Speaking of names and renaming. Perhaps `-f` is not the best choice here. `-f` options are conventionally used to control code generation parameters. `-foffload-*-only` mostly controls the driver behavior. Perhaps it would make sense to call them `--offload-host-only`, `--offload-device-only` and `--offload-host-device` and alias them to the matching `--cuda*` counterparts (or vice-versa + search/replace their use in the code). 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
