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

Josef Eisl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 10 07:19:09 PST 2019


zapster added a comment.

Thanks for your comment. I replied inline.



================
Comment at: llvm/lib/LTO/LTOBackend.cpp:335
+                       .Case("bitcode", EmbedBitcodeKind::Embed_Bitcode)
+                       .Case("marker", EmbedBitcodeKind::Embed_Marker)
+                       .Default(~0U);
----------------
tejohnson wrote:
> Does this value make any sense since CmdArgs is always empty below?
You are right. Currently, this option value is not utterly useful, but I kept it for compatibility with clangs `-fembed-bitcode`. It seems the marker section is needed (even if empty), otherwise certain tools will complain (notably the MacOS linker). Ideally, we would have something sensible to write into the section but I am not sure about reasonable values. I was not able to find any information about consumers of these commands. Pointers in that regard would be highly appreciated.

Anyhow, I lean toward keeping the options for compatibility with clang and for making it simple to properly implement the `marker` case. That said, I am perfectly fine with removing the option. In that case, I guess I should get rid of `EmbedBitcodeKind` altogether and always insert the bitcode and the marker, right?

On the other hand, we should maybe just consolidate the option (parsing) with the clang option. Some thoughts about that in my inline comment above.


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

https://reviews.llvm.org/D68213





More information about the llvm-commits mailing list