Generate extra .ll files before/after optimization when using -save-temps.

[jgorbe]

Summary

When -save-temps is specified, add two actions to the compilation to generate two new .ll outputs: one containing unoptimized IR and another one containing optimized IR.

Note that these new additional outputs will not be generated in compilations that target multiple architectures, because in that case the compiler generates an error when not all outputs generated by Actions can be processed by lipo.

[jgorbe]

Only generate extra .ll files when not targeting multiple archs. Updated related tests.

[tra]

lib/Driver/Driver.cpp

2599–2602

I'm not sure I understand why unoptimized IR can't be written out for multi-arch builds like CUDA. Could you elaborate?

[jgorbe]

Clarify that we only skip saving IR for multiarch Mach-O universal builds, not other multi-arch builds like CUDA.

lib/Driver/Driver.cpp

2599–2602

The error I was trying to work around here is this one.

It's not really specific to all multi-arch builds, but to Mach-O multi-arch builds. For those, BuildUniversalActions is called, which checks that all actions produce outputs with lipo-able types (see also the types::canLipoType() function) or errors out otherwise. I'm not sure the check is still necessary, but I got no answers when I asked about it in cfe-dev, so I decided to just not do it for that kind of builds.

I have updated the patch to make it clear that we only skip generating these IR files in that particular case, not every multi-arch build. Thanks for pointing this out!

[tra]

lib/Driver/Driver.cpp

2603–2614

Can this be moved below addHostDependenceToDeviceActions() on line 2626?
See my comment in cuda-options.cu below for the reasons why it may be necessary.

test/Driver/cuda-options.cu

197–202

It appears that the new actions you've pushed trigger at least parts of host-side compilation to happen before device-side compilation is done. That, at the very least, will not work well for CUDA. Compilation will probably succeed, but it will be missing device-side code and will fail at runtime.

If it's only host-side preprocessor that happens ahead of device-side actions, then may be OK, but in general host actions must be done after device's.

[jgorbe]

lib/Driver/Driver.cpp

2603–2614

I have tried but it didn't cause the test to go back to the previous ordering where all device-side actions are executed before all host-side actions.

I have noticed that, before applying my patch, the action graph produced by clang with -ccc-print-phases doesn't seem to introduce any explicit dependency to guarantee that device-side actions are executed before host-side actions:

0: input, "cuda-options.cu", cuda, (host-cuda)
1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
2: compiler, {1}, ir, (host-cuda)
3: input, "cuda-options.cu", cuda, (device-cuda, sm_20)
4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_20)
5: compiler, {4}, ir, (device-cuda, sm_20)
6: backend, {5}, assembler, (device-cuda, sm_20)
7: assembler, {6}, object, (device-cuda, sm_20)
8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_20)" {7}, object
9: offload, "device-cuda (nvptx64-nvidia-cuda:sm_20)" {6}, assembler
10: linker, {8, 9}, cuda-fatbin, (device-cuda)
11: offload, "host-cuda (x86_64--linux-gnu)" {2}, "device-cuda (nvptx64-nvidia-cuda)" {10}, ir
12: backend, {11}, assembler, (host-cuda)
13: assembler, {12}, object, (host-cuda)

I'm trying to figure out now if there's something else that enforces that restriction, or the current compilation order being the right one is a happy coincidence that my patch happened to disturb. I'm new to the project, so I'm working under the assumption that I'm missing something, any hints will be appreciated :)

include/clang/Driver/Driver.h

void BuildInputs(const ToolChain &TC, llvm::opt::DerivedArgList &Args,
                    InputList &Inputs) const;
 
   /// BuildActions - Construct the list of actions to perform for the
   /// given arguments, which are only done for a single architecture.
   ///
   /// \param C - The compilation that is being built.
   /// \param Args - The input arguments.
   /// \param Actions - The list to store the resulting actions onto.
+  /// \param MultiArchUniversalBuild - Whether a universal build for multiple
+  /// architectures is being performed.
   void BuildActions(Compilation &C, llvm::opt::DerivedArgList &Args,
-                    const InputList &Inputs, ActionList &Actions) const;
+                    const InputList &Inputs, ActionList &Actions,
+                    bool MultiArchUniversalBuild) const;
 
   /// BuildUniversalActions - Construct the list of actions to perform
   /// for the given arguments, which may require a universal build.
   ///
   /// \param C - The compilation that is being built.
   /// \param TC - The default host tool chain.
   void BuildUniversalActions(Compilation &C, const ToolChain &TC,
                              const InputList &BAInputs) const;

lib/Driver/Driver.cpp

Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   // Populate the tool chains for the offloading devices, if any.
   CreateOffloadingDeviceToolChains(*C, Inputs);
 
   // Construct the list of abstract actions to perform for this compilation. On
   // MachO targets this uses the driver-driver and universal actions.
   if (TC.getTriple().isOSBinFormatMachO())
     BuildUniversalActions(*C, C->getDefaultToolChain(), Inputs);
   else
-    BuildActions(*C, C->getArgs(), Inputs, C->getActions());
+    BuildActions(*C, C->getArgs(), Inputs, C->getActions(), false);
 
   if (CCCPrintPhases) {
     PrintActions(*C);
     return C;
   }
 
   BuildJobs(*C);
 
void Driver::generateCompilationDiagnostics(Compilation &C,
   }
 
   // Construct the list of abstract actions to perform for this compilation. On
   // Darwin OSes this uses the driver-driver and builds universal actions.
   const ToolChain &TC = C.getDefaultToolChain();
   if (TC.getTriple().isOSBinFormatMachO())
     BuildUniversalActions(C, TC, Inputs);
   else
-    BuildActions(C, C.getArgs(), Inputs, C.getActions());
+    BuildActions(C, C.getArgs(), Inputs, C.getActions(), false);
 
   BuildJobs(C);
 
   // If there were errors building the compilation, quit now.
   if (Trap.hasErrorOccurred()) {
     Diag(clang::diag::note_drv_command_failed_diag_msg)
         << "Error generating preprocessed source(s).";
     return;
void Driver::BuildUniversalActions(Compilation &C, const ToolChain &TC,
   }
 
   // When there is no explicit arch for this platform, make sure we still bind
   // the architecture (to the default) so that -Xarch_ is handled correctly.
   if (!Archs.size())
     Archs.push_back(Args.MakeArgString(TC.getDefaultUniversalArchName()));
 
   ActionList SingleActions;
-  BuildActions(C, Args, BAInputs, SingleActions);
+  BuildActions(C, Args, BAInputs, SingleActions, Archs.size() > 1);
 
   // Add in arch bindings for every top level action, as well as lipo and
   // dsymutil steps if needed.
   for (Action* Act : SingleActions) {
     // Make sure we can lipo this kind of output. If not (and it is an actual
     // output) then we disallow, since we can't create an output file with the
     // right name without overwriting it. We could remove this oddity by just
     // changing the output names to include the arch, which would also fix
OffloadAction::HostDependence HDep(
         *HostAction, *C.getSingleOffloadToolChain<Action::OFK_Host>(),
         /*BoundArch*/ nullptr, ActiveOffloadKinds);
     return C.MakeAction<OffloadAction>(HDep, DDeps);
   }
 };
 } // anonymous namespace.
 
 void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
-                          const InputList &Inputs, ActionList &Actions) const {
+                          const InputList &Inputs, ActionList &Actions,
+                          bool MultiArchUniversalBuild) const {
   llvm::PrettyStackTraceString CrashInfo("Building compilation actions");
 
   if (!SuppressMissingInputWarning && Inputs.empty()) {
     Diag(clang::diag::err_drv_no_input_files);
     return;
   }
 
   Arg *FinalPhaseArg;
for (SmallVectorImpl<phases::ID>::iterator i = PL.begin(), e = PL.end();
       // Queue linker inputs.
       if (Phase == phases::Link) {
         assert((i + 1) == e && "linking must be final compilation step.");
         LinkerInputs.push_back(Current);
         Current = nullptr;
         break;
       }
 
+      // When saving temps, add extra actions to write unoptimized and optimized
+      // IR besides the normal bitcode outputs if possible. This is not possible
+      // for Mach-O multi-arch universal builds because in these builds we check
+      // that we can lipo all action outputs, and types::TY_LLVM_IR is not
+      // lipo-able.
+      if (!MultiArchUniversalBuild) {
+        if (isSaveTempsEnabled() && Phase == phases::Compile) {
+          Actions.push_back(
+              C.MakeAction<CompileJobAction>(Current, types::TY_LLVM_IR));
+        }
+        if (isSaveTempsEnabled() && Phase == phases::Backend) {
+          Actions.push_back(
+              C.MakeAction<BackendJobAction>(Current, types::TY_LLVM_IR));
+        }
+      }
+
       // Otherwise construct the appropriate action.
       auto *NewCurrent = ConstructPhaseAction(C, Args, Phase, Current);
 
       // We didn't create a new action, so we will just move to the next phase.
       if (NewCurrent == Current)
         continue;
 
       Current = NewCurrent;
if (MultipleArchs && !BoundArch.empty()) {
       Suffixed.append(BoundArch);
     }
     // When using both -save-temps and -emit-llvm, use a ".tmp.bc" suffix for
     // the unoptimized bitcode so that it does not get overwritten by the ".bc"
     // optimized bitcode output.
     if (!AtTopLevel && C.getArgs().hasArg(options::OPT_emit_llvm) &&
         JA.getType() == types::TY_LLVM_BC)
       Suffixed += ".tmp";
+    // When using -save-temps, append a ".unoptimized" suffix so that the
+    // optimized .ll file doesn't overwrite the unoptimized one.
+    if (isSaveTempsEnabled() && JA.getType() == types::TY_LLVM_IR &&
+        JA.getKind() == Action::CompileJobClass)
+      Suffixed += ".unoptimized";
     Suffixed += '.';
     Suffixed += Suffix;
     NamedOutput = C.getArgs().MakeArgString(Suffixed.c_str());
   }
 
   // Prepend object file path if -save-temps=obj
   if (!AtTopLevel && isSaveTempsObj() && C.getArgs().hasArg(options::OPT_o) &&
       JA.getType() != types::TY_PCH) {

test/Driver/cuda-options.cu

 // ARCH-SM20: "-cc1"{{.*}}"-target-cpu" "sm_20"
 // NOARCH-SM20-NOT: "-cc1"{{.*}}"-target-cpu" "sm_20"
 // ARCH-SM30: "-cc1"{{.*}}"-target-cpu" "sm_30"
 // NOARCH-SM30-NOT: "-cc1"{{.*}}"-target-cpu" "sm_30"
 // ARCH-SM35: "-cc1"{{.*}}"-target-cpu" "sm_35"
 // NOARCH-SM35-NOT: "-cc1"{{.*}}"-target-cpu" "sm_35"
 // ARCHALLERROR: error: Unsupported CUDA gpu architecture: all
 
+// Match host-side preprocessor job with -save-temps.
+// HOST-SAVE: "-cc1" "-triple" "x86_64--linux-gnu"
+// HOST-SAVE-SAME: "-aux-triple" "nvptx64-nvidia-cuda"
+// HOST-SAVE-NOT: "-fcuda-is-device"
+// HOST-SAVE-SAME: "-x" "cuda"
+
 // Match device-side preprocessor and compiler phases with -save-temps.
 // DEVICE-SAVE: "-cc1" "-triple" "nvptx64-nvidia-cuda"
 // DEVICE-SAVE-SAME: "-aux-triple" "x86_64--linux-gnu"
 // DEVICE-SAVE-SAME: "-fcuda-is-device"
 // DEVICE-SAVE-SAME: "-x" "cuda"
 
 // DEVICE-SAVE: "-cc1" "-triple" "nvptx64-nvidia-cuda"
 // DEVICE-SAVE-SAME: "-aux-triple" "x86_64--linux-gnu"

 // NODEVICE-NOT: "-cc1" "-triple" "nvptx64-nvidia-cuda"
 // NODEVICE-NOT: "-fcuda-is-device"
 
 // INCLUDES-DEVICE:fatbinary
 // INCLUDES-DEVICE-DAG: "--create" "[[FATBINARY:[^"]*]]"
 // INCLUDES-DEVICE-DAG: "--image=profile=sm_{{[0-9]+}},file=[[CUBINFILE]]"
 // INCLUDES-DEVICE-DAG: "--image=profile=compute_{{[0-9]+}},file=[[PTXFILE]]"
 
-// Match host-side preprocessor job with -save-temps.
-// HOST-SAVE: "-cc1" "-triple" "x86_64--linux-gnu"
-// HOST-SAVE-SAME: "-aux-triple" "nvptx64-nvidia-cuda"
-// HOST-SAVE-NOT: "-fcuda-is-device"
-// HOST-SAVE-SAME: "-x" "cuda"
-
 // Match host-side compilation.
 // HOST: "-cc1" "-triple" "x86_64--linux-gnu"
 // HOST-SAME: "-aux-triple" "nvptx64-nvidia-cuda"
 // HOST-NOT: "-fcuda-is-device"
 // HOST-SAME: "-o" "[[HOSTOUTPUT:[^"]*]]"
 // HOST-NOSAVE-SAME: "-x" "cuda"
 // HOST-SAVE-SAME: "-x" "cuda-cpp-output"
 // INCLUDES-DEVICE-SAME: "-fcuda-include-gpubinary" "[[FATBINARY]]"

test/Driver/save-temps.c

 // RUN: %clang -target x86_64-apple-darwin -save-temps -arch x86_64 %s -### 2>&1 \
 // RUN:   | FileCheck %s
 // CHECK: "-o" "save-temps.i"
+// CHECK: "-o" "save-temps.unoptimized.ll"
 // CHECK: "-emit-llvm-uselists"
 // CHECK: "-disable-llvm-passes"
 // CHECK: "-o" "save-temps.bc"
+// CHECK: "-o" "save-temps.ll"
 // CHECK: "-o" "save-temps.s"
 // CHECK: "-o" "save-temps.o"
 // CHECK: "-o" "a.out"
 
 // Check -save-temps=cwd which should work the same as -save-temps above
 //
 // RUN: %clang -target x86_64-apple-darwin -save-temps=cwd -arch x86_64 %s -### 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CWD