[PATCH] D87949: [ThinLTO] Option to bypass function importing.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Minor comments.



================
Comment at: clang/test/CodeGen/thinlto_embed_bitcode.ll:17
+; RUN: rm %t1.bc %t2.bc
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir %t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt -mllvm -thinlto-assume-merged
+; RUN: diff %t-redo.o %t.o
----------------
Can we do this clang invocation and diff later on so you don't need to redo the opt and llvm-lto steps below?


================
Comment at: llvm/include/llvm/LTO/LTOBackend.h:59
+
+/// Load the referenced modules, keeping their buffers alive in the provided
+/// OwnedImportLifetimeManager. Returns false if the operation failed.
----------------
Note this is for distributed ThinLTO.


================
Comment at: llvm/lib/LTO/LTOBackend.cpp:75
+    cl::desc(
+        "Assume the input has already undergone ThinLTO function importing."));
+
----------------
Nit on name/comment. It isn't just thinlto importing (also things like linkage type changes). I'm not sure of a concise name to represent this though. Because we do apply some ThinLTO optimizations from the index later (WPD). Maybe just expand the description to something like "ThinLTO function importing and other pre-optimization pipeline changes."?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87949