tra added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6223-6224
if (IsCuda || IsHIP) {
- if (Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false))
+ if (Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false) ||
+ Args.hasArg(options::OPT_foffload_new_driver))
CmdArgs.push_back("-fgpu-rdc");
----------------
tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > If user specifies both `-fno-gpu-rdc` and `-foffload-new-driver` we would
> > > still enable RDC compilation.
> > > We may want to at least issue a warning.
> > >
> > > Considering that we have multiple places where we may check for
> > > `-f[no]gpu-rdc` we should make sure we don't get different ideas whether
> > > RDC has been enabled.
> > > I think it may make sense to provide a common way to figure it out.
> > > Either via a helper function that would process CLI arguments or
> > > calculate it once and save it somewhere.
> > I haven't quite finalized how to handle this. The new driver should be
> > compatible with a non-RDC build since we simply wouldn't embed the device
> > image or create offloading entries. It's a little bit more difficult here
> > since the new method is opt-in so it requires a flag. We should definitely
> > emit a warning if both are enabled (I'm assuming there's one for passing
> > both `fgpu-rdc` and `fno-gpu-rdc`). I'll add one in.
> >
> > Also we could consider the new driver *the* RDC in the future which would
> > be the easiest. The problem is if we want to support CUDA's method of RDC
> > considering how other build systems seem to expect it. I could see us
> > embedding the fatbinary in the object file, even if unused, just so that
> > cuobjdump works. However we couldn't support the generation of
> > `__cudaRegisterFatBinary_nv....` functions because then those would cause
> > linker errors. WDYT?
> > I'm assuming there's one for passing both fgpu-rdc and fno-gpu-rdc
>
> This is not a valid assumption. The whole idea behind `-fno-something` is
> that the options can be overridden. E.g. if the build specifies a standard
> set of compiler options, but we need to override some of them when building a
> particular file. We can only do so by appending to the standard options.
> Potentially we may end up having those options overridden again. While it's
> not very common, it's certainly possible. It's also possible to start with
> '-fno-gpu-rdc' and then override it with `-fgpu-rdc`.
>
> In this case, we care about the final state of RDC specified by -f*gpu-rdc
> options, not by the fact that `-fno-gpu-rdc` is present.
> `Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false)` does
> exactly that -- gives you the final state. If it returns false, but we have
> `-foffload-new-driver`, then we need a warning as these options are
> contradictory.
>
> The new driver should be compatible with a non-RDC build
In that case, we don't need a warning, but we do need a test verifying this
behavior.
> Also we could consider the new driver *the* RDC in the future which would be
> the easiest.
SGTM. I do not know how it all will work out in the end. Your proposed model
makes a lot of sense, and I'm guardedly optimistic about it.
Eventually we would deprecate RDC options, but we still need to work sensibly
when user specifies a mix of these options.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120272/new/
https://reviews.llvm.org/D120272
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
