[PATCH] D17392: Embed bitcode in object file (clang cc1 part)

[Steven Wu via cfe-commits]

steven_wu added inline comments.

================
Comment at: lib/CodeGen/BackendUtil.cpp:769-770
@@ +768,4 @@
+void clang::EmbedBitcode(llvm::Module *M, const CodeGenOptions &CGOpts,
+                         llvm::MemoryBufferRef Buf)
+{
+  if (CGOpts.getEmbedBitcode() == CodeGenOptions::Embed_Off)
----------------
rsmith wrote:
> Ping.
Sorry, I missed this. Will fix.

================
Comment at: lib/CodeGen/BackendUtil.cpp:774-806
@@ +773,35 @@
+
+  // Embed the bitcode for the llvm module.
+  std::string Data;
+  ArrayRef<uint8_t> ModuleData;
+  Triple T(M->getTargetTriple());
+  if (CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Marker) {
+    if (!isBitcode((const unsigned char *)Buf.getBufferStart(),
+                   (const unsigned char *)Buf.getBufferEnd())) {
+      // If the input is LLVM Assembly, bitcode is produced by serializing
+      // the module. Use-lists order need to be perserved in this case.
+      llvm::raw_string_ostream OS(Data);
+      llvm::WriteBitcodeToFile(M, OS, /* ShouldPreserveUseListOrder */ true);
+      ModuleData =
+          ArrayRef<uint8_t>((const uint8_t *)OS.str().data(), OS.str().size());
+    } else
+      // If the input is LLVM bitcode, write the input byte stream directly.
+      ModuleData = ArrayRef<uint8_t>((const uint8_t *)Buf.getBufferStart(),
+                                     Buf.getBufferSize());
+  }
+  llvm::Constant *ModuleConstant =
+      llvm::ConstantDataArray::get(M->getContext(), ModuleData);
+  // Use Appending linkage so it doesn't get optimized out.
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+      *M, ModuleConstant->getType(), true, llvm::GlobalValue::AppendingLinkage,
+      ModuleConstant);
+  GV->setSection(getSectionNameForBitcode(T));
+  if (llvm::GlobalVariable *Old =
+          M->getGlobalVariable("llvm.embedded.module")) {
+    assert(Old->use_empty() && "llvm.embedded.module must have no uses");
+    GV->takeName(Old);
+    Old->eraseFromParent();
+  } else {
+    GV->setName("llvm.embedded.module");
+  }
+
----------------
rsmith wrote:
> Can you skip all this if `Buf` is empty (in the common case where the input is not an IR file)?
-fembed-bitcode-marker will actually take this path. -fembed-bitcode-marker will not split the compilation into two stages thus the input is not IR and Buf is empty but clang needs to generate a marker.

================
Comment at: lib/Driver/Tools.cpp:3766-3771
@@ -3765,8 +3765,8 @@
     // Add flags implied by -fembed-bitcode.
-    CmdArgs.push_back("-fembed-bitcode");
+    Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
     // Disable all llvm IR level optimizations.
     CmdArgs.push_back("-disable-llvm-optzns");
   }
   if (C.getDriver().embedBitcodeMarkerOnly())
-    CmdArgs.push_back("-fembed-bitcode-marker");
+    CmdArgs.push_back("-fembed-bitcode=marker");
 
----------------
rsmith wrote:
> This changes the semantics of `clang -fembed-bitcode -fembed-bitcode-marker` to embed only a marker and no bitcode. Is that OK?
It wasn't clearly defined so I am ok with changing the semantics. It makes even more sense that we have different embed options.

================
Comment at: lib/Driver/Tools.cpp:5724-5728
@@ -5723,4 +5723,7 @@
   // by the frontend.
-  if (C.getDriver().isSaveTempsEnabled() && isa<CompileJobAction>(JA))
+  // When -fembed-bitcode is enabled, optimized bitcode is emitted because it
+  // has slightly different breakdown between stages.
+  if (C.getDriver().isSaveTempsEnabled() &&
+      !C.getDriver().embedBitcodeEnabled() && isa<CompileJobAction>(JA))
     CmdArgs.push_back("-disable-llvm-passes");
 
----------------
rsmith wrote:
> Why is this necessary? `-fembed-bitcode` implies `-disable-llvm-optzns` anyway.
-disable-llvm-passes is for -save-temps option. -fembed-bitcode and -save-temps disable optzns differently.
-save-temps has following outputs: preprocess source, IR emitted from front end, assembly file, object file
-fembed-bitcode has following outputs: optimized IR, object file
They don't share the same intermediate output at all so they need to use -disable-llvm-optzns at different stages.



http://reviews.llvm.org/D17392