[ThinLTO] Make -lto-embed-bitcode an enum

[mtrofin]

Summary

The current behavior of -lto-embed-bitcode is not quite the same as that
of -fembed-bitcode. While both populate .llvmbc with bitcode, the latter
populates it with pre-optimized bitcode(*), while the former with
post-optimized. The scenarios driving them are different - the latter's
goal is to allow re-compilation, while the former, IIUC, is execution.

I plan to add a third mode for thinlto cases, closely-related to
-fembed-bitcode's scenario: adding the bitcode pre-optimization, but
post-merging. This would allow re-compilation without requiring the
other .bc files that were merged (akin to how -fembed-bitcode allows
recompilation without all the .h files)

The third mode can't co-exist with the current -lto-embed-bitcode mode,
because the latter would overwrite it. For clarity, we change
-lto-embed-bitcode to be an enum.

(*) That's the compiler semantics. The driver splits compilation in 2
phases, so if -fembed-bitcode is given to the driver, the .llvmbc is
optimized bitcode; if the option is passed to the compiler (after -cc1),
the section is pre-optimized.

[tejohnson]

The change itself looks good to me, but I don't understand this comment from the summary:

(*) That's the compiler semantics. The driver splits compilation in 2 phases, so if -fembed-bitcode is given to the driver, the .llvmbc is optimized bitcode; if the option is passed to the compiler (after -cc1), the section is pre-optimized.

Are you saying that "clang -fembed-bitcode foo.c -O2 -c" does something different than "clang -cc1 -fembed-bitcode foo.c -O2 -c". That seems strange and broken to me. I thought the intent of the option is to emit pre-optimized code.

[mtrofin]

In D87477#2268461, @tejohnson wrote:

The change itself looks good to me, but I don't understand this comment from the summary:

(*) That's the compiler semantics. The driver splits compilation in 2 phases, so if -fembed-bitcode is given to the driver, the .llvmbc is optimized bitcode; if the option is passed to the compiler (after -cc1), the section is pre-optimized.

Are you saying that "clang -fembed-bitcode foo.c -O2 -c" does something different than "clang -cc1 -fembed-bitcode foo.c -O2 -c". That seems strange and broken to me. I thought the intent of the option is to emit pre-optimized code.

That's indeed how it works. I was also under the same impression (by looking at the code). The semantic nuance can be derived from the RFC (specifically, http://lists.llvm.org/pipermail/llvm-dev/2016-February/094907.html), albeit, TBH, it took me a bit to put the 2 together (i.e. what the RFC said with what I saw in code).

[steven_wu]

SGTM.

[tejohnson]

LGTM

[zapster]

@mtrofin this change (and related changes in other reviews) look good to me! I'm currently on parental leave so I am no able to review in a timely manner. However, I'd appreciate if you could keep me in the loop for -lto-embed-bitcode related changes. Thanks in advance!

llvm/lib/LTO/LTOBackend.cpp

 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/Scalar/LoopPassManager.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 #include "llvm/Transforms/Utils/SplitModule.h"
 
 using namespace llvm;
 using namespace lto;
 
+enum class LTOBitcodeEmbedding {
+  DoNotEmbed = 0,
+  EmbedOptimized = 1,
+};
+
+static cl::opt<LTOBitcodeEmbedding> EmbedBitcode(
+    "lto-embed-bitcode", cl::init(LTOBitcodeEmbedding::DoNotEmbed),
+    cl::values(clEnumValN(LTOBitcodeEmbedding::DoNotEmbed, "none",
+                          "Do not embed"),
+               clEnumValN(LTOBitcodeEmbedding::EmbedOptimized, "optimized",
+                          "Embed after all optimization passes")),
+    cl::desc("Embed LLVM bitcode in object files produced by LTO"));
+
 LLVM_ATTRIBUTE_NORETURN static void reportOpenError(StringRef Path, Twine Msg) {
   errs() << "failed to open " << Path << ": " << Msg << '\n';
   errs().flush();
   exit(1);
 }
 
 Error Config::addSaveTemps(std::string OutputFileName,
                            bool UseInputModulePath) {
bool opt(const Config &Conf, TargetMachine *TM, unsigned Task, Module &Mod,
   else if (Conf.UseNewPM)
     runNewPMPasses(Conf, Mod, TM, Conf.OptLevel, IsThinLTO, ExportSummary,
                    ImportSummary);
   else
     runOldPMPasses(Conf, Mod, TM, IsThinLTO, ExportSummary, ImportSummary);
   return !Conf.PostOptModuleHook || Conf.PostOptModuleHook(Task, Mod);
 }
 
-static cl::opt<bool> EmbedBitcode(
-    "lto-embed-bitcode", cl::init(false),
-    cl::desc("Embed LLVM bitcode in object files produced by LTO"));
-
-static void EmitBitcodeSection(Module &M) {
-  if (!EmbedBitcode)
-    return;
-  llvm::EmbedBitcodeInModule(M, llvm::MemoryBufferRef(), /*EmbedBitcode*/ true,
-                             /*EmbedMarker*/ false, /*CmdArgs*/ nullptr);
-}
-
 void codegen(const Config &Conf, TargetMachine *TM, AddStreamFn AddStream,
              unsigned Task, Module &Mod,
              const ModuleSummaryIndex &CombinedIndex) {
   if (Conf.PreCodeGenModuleHook && !Conf.PreCodeGenModuleHook(Task, Mod))
     return;
 
-  EmitBitcodeSection(Mod);
+  if (EmbedBitcode == LTOBitcodeEmbedding::EmbedOptimized)
+    llvm::EmbedBitcodeInModule(Mod, llvm::MemoryBufferRef(),
+                               /*EmbedBitcode*/ true,
+                               /*EmbedMarker*/ false, /*CmdArgs*/ nullptr);
 
   std::unique_ptr<ToolOutputFile> DwoOut;
   SmallString<1024> DwoFile(Conf.SplitDwarfOutput);
   if (!Conf.DwoDir.empty()) {
     std::error_code EC;
     if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir))
       report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " +
                          EC.message());

llvm/test/LTO/X86/embed-bitcode.ll

 ; RUN: llvm-as %s -o %t1.o
 ; RUN: llvm-as %p/Inputs/start-lib1.ll -o %t2.o
 ; RUN: llvm-as %p/Inputs/start-lib2.ll -o %t3.o
 
 ; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -o %t3 %t1.o %t2.o %t3.o
 ; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
 
-; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -lto-embed-bitcode=false -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -lto-embed-bitcode=none -o %t3 %t1.o %t2.o %t3.o
 ; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
 
-; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -lto-embed-bitcode -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,lx -lto-embed-bitcode=optimized -o %t3 %t1.o %t2.o %t3.o
 ; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t3.0 /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefix=CHECK-LL
 
 ; CHECK-ELF:      .text   PROGBITS 0000000000000000 [[#%x,OFF:]] [[#%x,SIZE:]] 00 AX 0
 ; CHECK-ELF-NEXT: .llvmbc PROGBITS 0000000000000000 [[#%x,OFF:]] [[#%x,SIZE:]] 00    0
 
 ; CHECK-LL: @_start