[PATCH] D74132: [ThinLTO/WPD] Remove dead code and improve test case

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 created this revision.
evgeny777 added a reviewer: tejohnson.
Herald added subscribers: dexonsmith, steven_wu, hiraditya, inglorion, mehdi_amini.
Herald added a project: LLVM.

I was studying changes to BackendUtil introduced by D71913 <https://reviews.llvm.org/D71913> and found out that

- Removing invocation of LTT pass in BackendUtil.cpp doesn't break any test case including `thinlto-distributed-type-metadata.cpp`
- When combined index doesn't have `skipModuleByDistributedBackend`flag  we run normal thin LTO pipeline which removes type.test/assume sequences either in WPD or in LTT.
- When combined index does have `skipModuleByDistributedBackend` flag we end up with empty module without invoking backend.

So adding LTT pass in BackendUtil seems to be not needed.


https://reviews.llvm.org/D74132

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp
  llvm/tools/llvm-lto/llvm-lto.cpp


Index: llvm/tools/llvm-lto/llvm-lto.cpp
===================================================================
--- llvm/tools/llvm-lto/llvm-lto.cpp
+++ llvm/tools/llvm-lto/llvm-lto.cpp
@@ -99,6 +99,10 @@
     ThinLTO("thinlto", cl::init(false),
             cl::desc("Only write combined global index for ThinLTO backends"));
 
+static cl::opt<bool> SkipModuleForDistributedBackend(
+    "skip-module-for-distributed-backend", cl::init(false),
+    cl::desc("Request distributed backend to skip compilation of the module"));
+
 enum ThinLTOModes {
   THINLINK,
   THINDISTRIBUTE,
@@ -417,6 +421,8 @@
         ExitOnErr(errorOrToExpected(MemoryBuffer::getFileOrSTDIN(Filename)));
     ExitOnErr(readModuleSummaryIndex(*MB, CombinedIndex, NextModuleId++));
   }
+  if (SkipModuleForDistributedBackend)
+    CombinedIndex.setSkipModuleByDistributedBackend();
   std::error_code EC;
   assert(!OutputFilename.empty());
   raw_fd_ostream OS(OutputFilename + ".thinlto.bc", EC,
Index: clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp
===================================================================
--- clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp
+++ clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp
@@ -10,6 +10,11 @@
 // RUN: %clang -target x86_64-unknown-linux -O2 -o %t3.o -x ir %t.o -c -fthinlto-index=%t2.thinlto.bc -save-temps=obj
 // RUN: llvm-dis %t.s.4.opt.bc -o - | FileCheck --check-prefix=OPT %s
 // llvm-nm %t3.o | FileCheck --check-prefix=NM %s
+// In case we skip module in a distributed backend we should end up with an empty module,
+// which doesn't have type.test/assume sequences.
+// RUN: llvm-lto -thinlto -skip-module-for-distributed-backend -o %t-skip %t.o
+// RUN: %clang -target x86_64-unknown-linux -O2 -o %t-empty.ll -x ir %t.o -S -emit-llvm -fthinlto-index=%t-skip.thinlto.bc
+// RUN: cat %t-empty.ll | FileCheck --check-prefix=EMPTY %s
 
 // The pre-link bitcode produced by clang should contain a type test assume
 // sequence.
@@ -31,6 +36,9 @@
 
 // NM: T _Z2afP1A
 
+// EMPTY-NOT: @llvm.type.test
+// EMPTY-NOT: call void @llvm.assume
+
 // Also check type test are lowered when the distributed ThinLTO backend clang
 // invocation is passed an empty index file, in which case a non-ThinLTO
 // compilation pipeline is invoked. If not lowered then LLVM CodeGen may assert.
Index: clang/lib/CodeGen/BackendUtil.cpp
===================================================================
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -554,16 +554,6 @@
   std::unique_ptr<TargetLibraryInfoImpl> TLII(
       createTLII(TargetTriple, CodeGenOpts));
 
-  // If we reached here with a non-empty index file name, then the index file
-  // was empty and we are not performing ThinLTO backend compilation (used in
-  // testing in a distributed build environment). Drop any the type test
-  // assume sequences inserted for whole program vtables so that codegen doesn't
-  // complain.
-  if (!CodeGenOpts.ThinLTOIndexFile.empty())
-    MPM.add(createLowerTypeTestsPass(/*ExportSummary=*/nullptr,
-                                     /*ImportSummary=*/nullptr,
-                                     /*DropTypeTests=*/true));
-
   PassManagerBuilderWrapper PMBuilder(TargetTriple, CodeGenOpts, LangOpts);
 
   // At O0 and O1 we only run the always inliner which is more efficient. At
@@ -1130,10 +1120,6 @@
       // (used in testing in a distributed build environment). Drop any the type
       // test assume sequences inserted for whole program vtables so that
       // codegen doesn't complain.
-      if (!CodeGenOpts.ThinLTOIndexFile.empty())
-        MPM.addPass(LowerTypeTestsPass(/*ExportSummary=*/nullptr,
-                                       /*ImportSummary=*/nullptr,
-                                       /*DropTypeTests=*/true));
       if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts))
         MPM.addPass(GCOVProfilerPass(*Options));
       if (Optional<InstrProfOptions> Options =


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74132.242903.patch
Type: text/x-patch
Size: 4008 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200206/34047989/attachment.bin>