[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

[Josef Eisl via Phabricator via llvm-commits]

zapster marked 5 inline comments as done.
zapster added a comment.

Added inline remarks.



================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1547
 }
 
-static const char* getSectionNameForBitcode(const Triple &T) {
----------------
moved to `llvm/lib/Bitcode/Writer/BitcodeWriter.cpp`


================
Comment at: clang/test/Frontend/x86-embed-bitcode.ll:1
+; REQUIRES: x86-registered-target
+; check .ll input
----------------
This duplicates the `embed-bitcode.ll` test (which only runs on ARM) for x86.


================
Comment at: llvm/include/llvm/Bitcode/BitcodeWriter.h:158
+                            bool EmbedMarker,
+                            const std::vector<uint8_t> *CmdArgs);
+
----------------
`BitcodeWriter.h` seems like a natural place for this functionality. However, suggestions for a better location are more than appreciated. 


================
Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4670
 }
+
+static const char *getSectionNameForBitcode(const Triple &T) {
----------------
moved from `clang/lib/CodeGen/BackendUtil.cpp`


================
Comment at: llvm/lib/LTO/LTOBackend.cpp:327
+
+static EmbedBitcodeKind getEmbedBitcode(Config &Conf) {
+  if (EmbedBitcode.empty())
----------------
This options parsing logic is duplicated from clang. We might want move this to a shared place, but I failed to find a good candidate. `include/llvm/Support/CodeGen.h` came to mind, but it currently only contains types, no code. Any suggestions?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68213/new/

https://reviews.llvm.org/D68213