[ThinLTO] Fix empty .llvmcmd sections

[mtrofin]

Summary

When passing -lto-embed-bitcode=post-merge-pre-opt, we were getting
empty .llvmcmd sections. It turns out that is because the
CodeGenOptions::CmdArgs field was only populated when clang saw
-fembed-bitcode={all|marker}.

This patch always populates the CodeGenOptions::CmdArgs. The overhead
of carrying through in memory in all cases is likely negligible in
the grand schema of things, and it keeps the using code simple.

[tejohnson]

The motivation and effects of the change are unclear to me. Does this mean that the .llvmcmd section is always emitted? Or always emitted whenever any bitcode embedding is requested?

clang/test/CodeGen/thinlto_embed_bitcode.ll

28

Incomplete sentence

[mtrofin]

In D90366#2362052, @tejohnson wrote:

The motivation and effects of the change are unclear to me. Does this mean that the .llvmcmd section is always emitted? Or always emitted whenever any bitcode embedding is requested?

This just means the arguments are captured in memory. Nothing else changes. The only penalty is that a char vector (the captured args) are carried around even if .llvmcmd wasn't generated.

We could maybe add a test that no explicit optin to emitting .llvmcmd => no section (briefly looked, doesn't seem to exist for either non- or thinlto)

[mtrofin]

fixed sentence

[mtrofin]

updated description

[tejohnson]

LGTM

clang/lib/Frontend/CompilerInvocation.cpp

if (Model == ~0U) {
       Success = false;
     } else
       Opts.setEmbedBitcode(
           static_cast<CodeGenOptions::EmbedBitcodeKind>(Model));
   }
   // FIXME: For backend options that are not yet recorded as function
   // attributes in the IR, keep track of them so we can embed them in a
   // separate data section and use them when building the bitcode.
-  if (Opts.getEmbedBitcode() == CodeGenOptions::Embed_All) {
-    for (const auto &A : Args) {
-      // Do not encode output and input.
-      if (A->getOption().getID() == options::OPT_o ||
-          A->getOption().getID() == options::OPT_INPUT ||
-          A->getOption().getID() == options::OPT_x ||
-          A->getOption().getID() == options::OPT_fembed_bitcode ||
-          A->getOption().matches(options::OPT_W_Group))
-        continue;
-      ArgStringList ASL;
-      A->render(Args, ASL);
-      for (const auto &arg : ASL) {
-        StringRef ArgStr(arg);
-        Opts.CmdArgs.insert(Opts.CmdArgs.end(), ArgStr.begin(), ArgStr.end());
-        // using \00 to separate each commandline options.
-        Opts.CmdArgs.push_back('\0');
-      }
-    }
-  }
 
   Opts.PreserveVec3Type = Args.hasArg(OPT_fpreserve_vec3_type);
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
   Opts.InstrumentFunctionsAfterInlining =
       Args.hasArg(OPT_finstrument_functions_after_inlining);
   Opts.InstrumentFunctionEntryBare =
       Args.hasArg(OPT_finstrument_function_entry_bare);
 

clang/test/CodeGen/thinlto_embed_bitcode.ll

 ; RUN: llvm-readelf -S %t-opt.o | FileCheck %s --check-prefixes=CHECK-ELF,CHECK-ELF-NO-CMD
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t-opt.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefixes=CHECK,CHECK-OPT
 
 ; For the post-merge case, perform the embedded bitcode extraction, then
 ; round-trip through compilation and ensure the objects are the same.
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t.o -x ir %t1.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt
 ; RUN: llvm-readelf -S %t.o | FileCheck %s --check-prefixes=CHECK-ELF,CHECK-ELF-CMD
+; RUN: llvm-objcopy --dump-section=.llvmcmd=%t-embedded.cmd %t.o /dev/null
+; RUN: grep x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOOPT
 ; We should only need the index and the post-thinlto merged module to generate 
 ; the exact same .o as we originally did.
 ; RUN: rm %t1.bc %t2.bc
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir %t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt -mllvm -thinlto-assume-merged
+;
+; The resulting .o is almost the same, but the .llvmcmd section would be
+; different because of the extra -thinlto-assume-merged.
+; A simple, meaningful comparison is to just compare the stripped objects.
+; RUN: llvm-strip --strip-all %t-redo.o 
+; RUN: llvm-strip --strip-all %t.o 
 ; RUN: diff %t-redo.o %t.o
 
 ; 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-ELF-CMD:  .llvmcmd
 ; CHECK-ELF-NO-CMD-NOT: .llvmcmd
 
 ; CHECK:          define void @foo() 
 ; CHECK-OPT-NEXT:   ret void
 ; CHECK-NOOPT-NEXT: call void @bar()
 ; CHECK-NOOPT: define available_externally void @bar()
 ; CHECK-NOOPT-NEXT: ret void