[PATCH] D146776: [llvm] Preliminary fat-lto-objects support

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/docs/FatLTOSupport.rst:1
+===================
+FatLTO Support
----------------
Maybe just `FatLTO.rst`. For other features, we don't use the `Support` suffix.


================
Comment at: llvm/docs/FatLTOSupport.rst:18
+compilers, like `GCC
+<https://gcc.gnu.org/onlinedocs/gccint/LTO-Overview.html>`_ for some time.
+
----------------



================
Comment at: llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp:32
+  if (M.getGlobalVariable("llvm.embedded.module", /*AllowInternal=*/true))
+    report_fatal_error("Can only embed the module once.",
+                       /*gen_crash_diag=*/false);
----------------
Nit: diagnostics do not need capitalization (https://llvm.org/docs/CodingStandards.html#error-and-warning-messages) but `report_fatal_error` messages often do capitalization, so I think capitalization is fine, but the period is often omitted.


================
Comment at: llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp:55
+  StringRef ModuleData(OS.str().data(), OS.str().size());
+  MemoryBufferRef Buf(ModuleData, "ModuleData");
+  embedBufferInModule(M, Buf, ".llvm.lto");
----------------
You can delete ModuleData and even Buf.


================
Comment at: llvm/lib/Passes/PassRegistry.def:175
+                         "EmbedBitcodePass",
+                        [](EmbedBitcodeOptions Opts){
+                          return EmbedBitcodePass(Opts);
----------------



================
Comment at: llvm/test/Bitcode/embed.ll:9
+; CHECK: @a = global i32 1
+;; Make sure the module is in the correct section
+; CHECK: @llvm.embedded.object = private constant {{.*}}, section ".llvm.lto", align 1
----------------
Nit: complete sentences in comments end with a period.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146776