[OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

[jhuber6]

Summary

Temporary files created by the offloading device toolchain are not removed
after compilation when using a two-step compilation. The offload-bundler uses a
different filename for the device binary than the .o file present in the
Job's input list. This is not listed as a temporary file so it is never
removed. This patch explicitly adds the device binary as a temporary file to
consume it. This fixes PR50336.

[jdoerfert]

Why the else?

[jhuber6]

In D107668#2937230, @jdoerfert wrote:

Why the else?

If this input isn't an OffloadingAction then it is a .o file which we already list as a temporary file. If it was an OffloadingAction then we have an extra device-specific filename like .cubin that we don't automatically list as a temporary file.

[jdoerfert]

LG

[yaxunl]

This may break -save-temps since the input to clang-offload-bundler may not be temporary files when -save-temps is enabled.

I think clang-offload-bundler is not the right place to decide whether a file is a temporary file. Whether a file is a temporary file should be determined at its point of creation and if it is a temporary file it should be addTempFile there, instead of guessing that later.

[jhuber6]

In D107668#2939365, @yaxunl wrote:

This may break -save-temps since the input to clang-offload-bundler may not be temporary files when -save-temps is enabled.

I think clang-offload-bundler is not the right place to decide whether a file is a temporary file. Whether a file is a temporary file should be determined at its point of creation and if it is a temporary file it should be addTempFile there, instead of guessing that later.

The .cubin files are still present when I tested it with save-temps. We already do something similar in the OpenMPLinker job for the Cuda driver, which is why these files were removed for straight compilation but kept when using the offload bundler when compiling with -c.

[Naghasan]

Any rational for not calling addTempFile during the binding rather than the call setup ? I had a brief look there and it seems to me this is the root of the problem.

I got bitten by this patch in a downstream project where some of the files in the offload action are actual inputs and so got deleted.

[jhuber6]

In D107668#3464124, @Naghasan wrote:

Any rational for not calling addTempFile during the binding rather than the call setup ? I had a brief look there and it seems to me this is the root of the problem.

I got bitten by this patch in a downstream project where some of the files in the offload action are actual inputs and so got deleted.

Sorry to hear that. If I recall the main problem was that the bindings always use the .o suffix while CUDA requires the .cubin suffix to work. There was a similar hack somewhere else so I just copied it here and didn't think too much about it. If you have a solution that solves your problem I could take a look at it. However, we're moving away from this tool anyway, upstream it's no longer used for OpenMP offloading and I'm currently working on making that true for HIP as well.

[Naghasan]

Thanks for your answer, make sense.

If we are moving away I won't bother looking fixing my issue upstream as it is very niche (I don't think you can trigger this with the upstream clang driver).

clang/lib/Driver/ToolChains/Clang.cpp

for (unsigned I = 0; I < Inputs.size(); ++I) {
     // Find ToolChain for this input.
     const ToolChain *CurTC = &getToolChain();
     if (const auto *OA = dyn_cast<OffloadAction>(JA.getInputs()[I])) {
       CurTC = nullptr;
       OA->doOnEachDependence([&](Action *, const ToolChain *TC, const char *) {
         assert(CurTC == nullptr && "Expected one dependence!");
         CurTC = TC;
       });
-    }
+      UB += C.addTempFile(
+          C.getArgs().MakeArgString(CurTC->getInputFilename(Inputs[I])));
+    } else {
       UB += CurTC->getInputFilename(Inputs[I]);
     }
+  }
   CmdArgs.push_back(TCArgs.MakeArgString(UB));
 
   // All the inputs are encoded as commands.
   C.addCommand(std::make_unique<Command>(
       JA, *this, ResponseFileSupport::None(),
       TCArgs.MakeArgString(getToolChain().GetProgramPath(getShortName())),
       CmdArgs, None, Output));
 }