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

Jonas Hahnfeld via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 20 09:07:31 PST 2017


Hahnfeld marked 2 inline comments as done.
Hahnfeld added inline comments.


================
Comment at: lib/Driver/ToolChains/Cuda.cpp:431
 
-    SmallString<256> Name(II.getFilename());
-    llvm::sys::path::replace_extension(Name, "cubin");
-
-    const char *CubinF =
-        C.addTempFile(C.getArgs().MakeArgString(Name));
+    const char *CubinF = C.addTempFile(
+        C.getArgs().MakeArgString(getToolChain().getInputFilename(II)));
----------------
gtbercea wrote:
> Hahnfeld wrote:
> > gtbercea wrote:
> > > Is this always a cubin?
> > Probably because the linker always takes object files...
> Considering that the function may also return an object file can you add an assert here that getInputFileName(II) returns a cubin in this case?
Hmm, that's a string. I've never seen asserts that say "this string should contain...". IMO that's what we have tests for


================
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);
----------------
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


https://reviews.llvm.org/D40250





More information about the cfe-commits mailing list