[LTO] Add ability to emit assembly to new LTO API

[tobiasvk]

Summary

Add a field to LTO::Config, CGFileType, to select the file type to emit (object
or assembly). This is useful for testing and to implement -save-temps.

[mehdi_amini]

Can you clarify how one would implement -save-temps in a linker this way?

[davide]

ehm, we already have -save-temps, why do you need this? (see what the gold-plugin and lld do)

[tobiasvk]

In D29475#665351, @mehdi_amini wrote:

Can you clarify how one would implement -save-temps in a linker this way?

Set Conf.CGFileType to CGFT_Assembly, then run an external assembler on all outputs produced by LTO before linking. We leave the output files around and output a notice to the user about their location. This is what we've been doing with the legacy API, and we'd like to keep it as we move to the new API.

[tobiasvk]

In D29475#665358, @davide wrote:

ehm, we already have -save-temps, why do you need this? (see what the gold-plugin and lld do)

It saves intermediate bitcode files (which is useful) but our customers also want to see assembly just like in the non-LTO flow.

[mehdi_amini]

In D29475#665360, @tobiasvk wrote:

In D29475#665351, @mehdi_amini wrote:

Can you clarify how one would implement -save-temps in a linker this way?

Set Conf.CGFileType to CGFT_Assembly, then run an external assembler on all outputs produced by LTO before linking. We leave the output files around and output a notice to the user about their location. This is what we've been doing with the legacy API, and we'd like to keep it as we move to the new API.

That'd require to have a driver to run the external assembler in the linker, which isn't very common right? Otherwise the build system would need to know about this, which is also a non-trivial integration. I guess an assembler does not need much setup in terms of flags to pass, but still it is not clear to me if we'd be able to get the same binary in the end. That'd seem quite "fragile" to me.
However for the LLVM side of it, the patch seems non-intrusive enough that it is fine to me.

@pcc, any objection?

[pcc]

I would echo Mehdi's comments: I think I would be surprised if -save-temps changed the output at all (it sounds like it would cause the linker to use a different assembler). If I were implementing this feature, I think I would consider alternatives first, such as making the "use an external assembler" feature orthogonal to -save-temps.

However, supporting a "use an external assembler" feature at the API level does not seem too burdensome. Can you please make a separate test in LTO/Resolution though?

[tobiasvk]

Move test to test/LTO/Resolution as per Peter's request.

Thank you all for your comments.

I agree about the need for an assembler 'driver' in the linker to make this
work and the potential fragility. This is what we have and it's worked fine on
our side with the previous LTO API. Note that clang's non-LTO -save-temps flow
calls the assembler separately (via clang -cc1as), too. It's perfectly doable
to ensure that the linker is called with the correct options to pass through to
the assembler when clang is used as the driver for LTO.

I like Rafael's idea of using the MC layer to emit both assembly and an object
at the same time with the same settings (if that's what you're proposing) - we
could use that for the non-LTO flow in clang too. I haven't checked how easy
that would be, but it's perhaps worth considering in the future.

Anyway, if there are no further objections, I'd like this to go in so the
legacy and resolution-based LTO APIs are feature-compatible in this respect.

[tobiasvk]

Ping?

[mehdi_amini]

LGTM

include/llvm/LTO/Config.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LTO_CONFIG_H
 #define LLVM_LTO_CONFIG_H
 
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 
 #include <functional>
 
 namespace llvm {
 
 class Error;
 class Module;
 class ModuleSummaryIndex;
 class raw_pwrite_stream;
 
 namespace lto {
 
 /// LTO configuration. A linker can configure LTO by setting fields in this data
 /// structure and passing it to the lto::LTO constructor.
 struct Config {
   // Note: when adding fields here, consider whether they need to be added to
   // computeCacheKey in LTO.cpp.
   std::string CPU;
   TargetOptions Options;
   std::vector<std::string> MAttrs;
   Reloc::Model RelocModel = Reloc::PIC_;
   CodeModel::Model CodeModel = CodeModel::Default;
   CodeGenOpt::Level CGOptLevel = CodeGenOpt::Default;
+  TargetMachine::CodeGenFileType CGFileType = TargetMachine::CGFT_ObjectFile;
   unsigned OptLevel = 2;
   bool DisableVerify = false;
 
   /// Disable entirely the optimizer, including importing for ThinLTO
   bool CodeGenOnly = false;
 
   /// If this field is set, the set of passes run in the middle-end optimizer
   /// will be the one specified by the string. Only works with the new pass

lib/LTO/LTO.cpp

#endif
   AddUnsigned(Conf.Options.FunctionSections);
   AddUnsigned(Conf.Options.DataSections);
   AddUnsigned((unsigned)Conf.Options.DebuggerTuning);
   for (auto &A : Conf.MAttrs)
     AddString(A);
   AddUnsigned(Conf.RelocModel);
   AddUnsigned(Conf.CodeModel);
   AddUnsigned(Conf.CGOptLevel);
+  AddUnsigned(Conf.CGFileType);
   AddUnsigned(Conf.OptLevel);
   AddString(Conf.OptPipeline);
   AddString(Conf.AAPipeline);
   AddString(Conf.OverrideTriple);
   AddString(Conf.DefaultTriple);
 
   // Include the hash for the current module
   auto ModHash = Index.getModuleHash(ModuleID);

lib/LTO/LTOBackend.cpp

 
 void codegen(Config &Conf, TargetMachine *TM, AddStreamFn AddStream,
              unsigned Task, Module &Mod) {
   if (Conf.PreCodeGenModuleHook && !Conf.PreCodeGenModuleHook(Task, Mod))
     return;
 
   auto Stream = AddStream(Task);
   legacy::PassManager CodeGenPasses;
-  if (TM->addPassesToEmitFile(CodeGenPasses, *Stream->OS,
-                              TargetMachine::CGFT_ObjectFile))
+  if (TM->addPassesToEmitFile(CodeGenPasses, *Stream->OS, Conf.CGFileType))
     report_fatal_error("Failed to setup codegen");
   CodeGenPasses.run(Mod);
 }
 
 void splitCodeGen(Config &C, TargetMachine *TM, AddStreamFn AddStream,
                   unsigned ParallelCodeGenParallelismLevel,
                   std::unique_ptr<Module> Mod) {
   ThreadPool CodegenThreadPool(ParallelCodeGenParallelismLevel);

test/LTO/Resolution/X86/asm-output.ll

+; Test the ability to emit assembly code from the resolution-based LTO API
+;
+; RUN: llvm-as < %s > %t1.bc
+;
+; RUN: llvm-lto2 -filetype=asm -r %t1.bc,main,px -o %t2 %t1.bc
+; RUN: FileCheck --check-prefix=ASM %s < %t2.0
+; RUN: llvm-lto2 -filetype=obj -r %t1.bc,main,px -o %t2 %t1.bc
+; RUN: llvm-objdump -d %t2.0 | FileCheck --check-prefix=ASM %s
+;
+; ASM: main:
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @main() {
+entry:
+  ret i32 23
+}
+

tools/llvm-lto2/llvm-lto2.cpp

int main(int argc, char **argv) {
   case '3':
     Conf.CGOptLevel = CodeGenOpt::Aggressive;
     break;
   default:
     llvm::errs() << "invalid cg optimization level: " << CGOptLevel << '\n';
     return 1;
   }
 
+  if (FileType.getNumOccurrences())
+    Conf.CGFileType = FileType;
+
   Conf.OverrideTriple = OverrideTriple;
   Conf.DefaultTriple = DefaultTriple;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
     Backend = createWriteIndexesThinBackend("", "", true, "");
   else
     Backend = createInProcessThinBackend(Threads);