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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to