[PATCH] D42995: [ThinLTO] Allow indexing to request backend to ignore the module

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1176
+      // Output of this module is not needed, but build system may not know
+      // that. So we need to generated empty valid object file.
+      EmptyModule = llvm::make_unique<llvm::Module>("empty", M->getContext());
----------------
s/generated/generate an/


================
Comment at: clang/test/CodeGen/thinlto-distributed-backend-skip.ll:3
+
+; Check that ThinLTO backed respects "SkipModule" flag which can be set by
+; indexing.
----------------
s/backed/backend/


================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:685
 
+  /// Indicates that corresponding module should be skipped.
+  bool SkipModule = false;
----------------
Need to document clearly that this only applies to the distributed index case, and what it means to "skip" the module (i.e. skip the backend compilation and generate an empty module instead). 

Maybe rename to something like SkipModuleDistributedBackend or something else very explicit to avoid confusion?


================
Comment at: llvm/tools/gold/gold-plugin.cpp:819
+                                              const std::string &NewPrefix,
+                                              bool Skip) {
   std::string NewModulePath =
----------------
Rename Skip to SkipModule to match the above comment documenting parameter


================
Comment at: llvm/tools/gold/gold-plugin.cpp:890
       addModule(*Lto, F, View, ObjFilename.first->first());
+    else {
+      ObjFilename.first->second = true;
----------------
Think this should be "else if (options::thinlto_index_only) {"
i.e. we don't need/want to write these when we're in single process mode.


================
Comment at: llvm/tools/gold/gold-plugin.cpp:892
+      ObjFilename.first->second = true;
+      writeEmptyDistributedBuildOutputs(Identifier, OldPrefix, NewPrefix, true);
+    }
----------------
Document constant parameters here and in other places. e.g. "/* SkipModule= */ true"


https://reviews.llvm.org/D42995