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