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

[Richard Smith via cfe-commits]

rsmith added a comment.

>From a code standpoint, this looks fine to me, but I'd like to understand a bit more about what you aim to achieve with the various parts here.


================
Comment at: include/clang/Frontend/CodeGenOptions.def:57
@@ -56,1 +56,3 @@
+CODEGENOPT(EmbedBitcode      , 1, 0) ///< Embed LLVM IR bitcode as data.
+CODEGENOPT(EmbedMarkerOnly   , 1, 0) ///< Only create bitcode section as marker
 CODEGENOPT(EmitDeclMetadata  , 1, 0) ///< Emit special metadata indicating what
----------------
What is this "marker only" mode for?

================
Comment at: lib/CodeGen/BackendUtil.cpp:752-753
@@ +751,4 @@
+void clang::EmbedBitcode(llvm::Module *M, const CodeGenOptions &CGOpts,
+                         llvm::MemoryBufferRef Buf)
+{
+  if (!CGOpts.EmbedBitcode && !CGOpts.EmbedMarkerOnly)
----------------
`{` on previous line.

================
Comment at: lib/CodeGen/BackendUtil.cpp:792
@@ +791,3 @@
+
+  // Embed command-line options.
+  ArrayRef<uint8_t> CmdData((uint8_t*)CGOpts.CmdArgs.data(),
----------------
As mentioned in the discussion on cfe-dev, embedding the command line and embedding bitcode appear to be completely orthogonal and separate concerns. Can you explain why it makes sense to control both with the same flag?

================
Comment at: lib/CodeGen/BackendUtil.cpp:793-794
@@ +792,4 @@
+  // Embed command-line options.
+  ArrayRef<uint8_t> CmdData((uint8_t*)CGOpts.CmdArgs.data(),
+                            CGOpts.CmdArgs.size());
+  llvm::Constant *CmdConstant =
----------------
This doesn't seem like you're saving enough information to be able to replay the build (you're missing the current working directory, for instance). Can you clarify exactly what you want to be able to do with the embedded command-line options?


http://reviews.llvm.org/D17392