[PATCH] D40250: [OpenMP] Consistently use cubin extension for nvlink

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: lib/Driver/ToolChains/Clang.cpp:5341-5344
+    if (const auto *OA = dyn_cast<OffloadAction>(JA.getInputs()[I])) {
+      OA->doOnEachDependence(
+          [&](Action *, const ToolChain *TC, const char *) { CurTC = TC; });
+    }
----------------
Can we ever have more than one dependence? If not, perhaps add an assert.
Otherwise, can dependencies have different toolchains? 
If so, which one do we really want?





================
Comment at: lib/Driver/ToolChains/Cuda.cpp:461
+std::string CudaToolChain::getInputFilename(const InputInfo &Input) const {
+  if (OK != Action::OFK_OpenMP || Input.getType() != types::TY_Object)
+    return ToolChain::getInputFilename(Input);
----------------
Hahnfeld wrote:
> gtbercea wrote:
> > Hahnfeld wrote:
> > > gtbercea wrote:
> > > > When does this situation occur?
> > > Well, if that condition fires:
> > > 1. For CUDA
> > > 2. For other types. For example, the bundler might need to bundle / unbundle assembly files.
> > Can you put this info in a comment just before the if statement?
> I'd say this condition is not the most difficult that I've ever seen, but can do
I usually find conditions expressed in positive terms are much easier to understand at a glance.
I.e. `if (!(OK == Action::OFK_OpenMP && Input.getType() == types::TY_Object))` is, IMO, almost obvious in its meaning, while the condition above required conscious effort to understand its purpose.


================
Comment at: lib/Driver/ToolChains/Cuda.h:144
 
+  virtual std::string getInputFilename(const InputInfo &Input) const override;
+
----------------
`virtual` is redundant here.


https://reviews.llvm.org/D40250