[LTO] Ignore regular LTO modules when looking for partially split LTO Units

[tejohnson]

Summary

LTO Unit splitting does not apply to regular LTO modules, only ThinLTO
(which must be consistently split into regular and Thin units for
optimizations such as whole program devirtualization and lower type
tests). Ignore non-Thin modules when looking for consistently split LTO
Units, to avoid spurious errors when linking split ThinLTO modules with
(non-split) regular LTO modules.

[tejohnson]

Going with a different clang-side approach (see D65009).

lib/LTO/LTO.cpp

 
 Error LTO::addModule(InputFile &Input, unsigned ModI,
                      const SymbolResolution *&ResI,
                      const SymbolResolution *ResE) {
   Expected<BitcodeLTOInfo> LTOInfo = Input.Mods[ModI].getLTOInfo();
   if (!LTOInfo)
     return LTOInfo.takeError();
 
+  BitcodeModule BM = Input.Mods[ModI];
+  auto ModSyms = Input.module_symbols(ModI);
+  addModuleToGlobalRes(ModSyms, {ResI, ResE},
+                       LTOInfo->IsThinLTO ? ThinLTO.ModuleMap.size() + 1 : 0,
+                       LTOInfo->HasSummary);
+
+  if (LTOInfo->IsThinLTO) {
+    // We only care about having consistently split ThinLTO modules, as regular
+    // LTO modules are always handled properly by whole program devirtualization
+    // and lower type tests, and we don't want to issue spurious errors linking
+    // with regular LTO modules.
     if (EnableSplitLTOUnit.hasValue()) {
       // If only some modules were split, flag this in the index so that
       // we can skip or error on optimizations that need consistently split
       // modules (whole program devirt and lower type tests).
       if (EnableSplitLTOUnit.getValue() != LTOInfo->EnableSplitLTOUnit)
         ThinLTO.CombinedIndex.setPartiallySplitLTOUnits();
     } else
       EnableSplitLTOUnit = LTOInfo->EnableSplitLTOUnit;
 
-  BitcodeModule BM = Input.Mods[ModI];
-  auto ModSyms = Input.module_symbols(ModI);
-  addModuleToGlobalRes(ModSyms, {ResI, ResE},
-                       LTOInfo->IsThinLTO ? ThinLTO.ModuleMap.size() + 1 : 0,
-                       LTOInfo->HasSummary);
-
-  if (LTOInfo->IsThinLTO)
     return addThinLTO(BM, ModSyms, ResI, ResE);
+  }
 
   Expected<RegularLTOState::AddedModule> ModOrErr =
       addRegularLTO(BM, ModSyms, ResI, ResE);
   if (!ModOrErr)
     return ModOrErr.takeError();
 
   if (!LTOInfo->HasSummary)
     return linkRegularLTO(std::move(*ModOrErr), /*LivenessFromIndex=*/false);

test/ThinLTO/X86/Inputs/nonthin_empty.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+^0 = module: (path: "nonthin.o", hash: (0, 0, 0, 0, 0))
+
+!0 = !{i32 1, !"ThinLTO", i32 0}
+!llvm.module.flags = !{ !0 }

test/ThinLTO/X86/Inputs/thin_empty.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+^0 = module: (path: "thin.o", hash: (0, 0, 0, 0, 0))

test/ThinLTO/X86/cfi-devirt.ll

 ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; REMARK: single-impl: devirtualized a call to _ZN1A1nEi
 
 ; Next check that we emit an error when trying to LTO link this module
 ; containing an llvm.type.checked.load (with a split LTO Unit) with one
 ; that does not have a split LTO Unit. Use -thinlto-distributed-indexes
 ; to ensure it is being caught in the thin link.
-; RUN: opt -thinlto-bc -o %t2.o %S/Inputs/empty.ll
+; RUN: opt -thinlto-bc -o %t2.o %S/Inputs/thin_empty.ll
 ; RUN: not llvm-lto2 run %t.o %t2.o -thinlto-distributed-indexes \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t.o,_ZN1C1fEi,p \
 ; RUN:   -r=%t.o,empty,p \
 ; RUN:   -r=%t.o,_ZTV1B, \
 ; RUN:   -r=%t.o,_ZTV1C, \
 ; RUN:   -r=%t.o,_ZN1A1nEi, \
 ; RUN:   -r=%t.o,_ZN1B1fEi, \
 ; RUN:   -r=%t.o,_ZN1C1fEi, \
 ; RUN:   -r=%t.o,_ZTV1B,px \
 ; RUN:   -r=%t.o,_ZTV1C,px 2>&1 | FileCheck %s --check-prefix=ERROR
 ; ERROR: failed: inconsistent LTO Unit splitting (recompile with -fsplit-lto-unit)
 
+; Simulate linking instead with a regular LTO module containing a summary
+; (not marked as split). This should succeed as we only care about
+; inconsistently split ThinLTO modules.
+; RUN: opt -module-summary -o %t4.o %S/Inputs/nonthin_empty.ll
+; RUN: llvm-lto2 run %t.o %t4.o -thinlto-distributed-indexes \
+; RUN:   -o %t3 \
+; RUN:   -r=%t.o,test,px \
+; RUN:   -r=%t.o,_ZN1A1nEi,p \
+; RUN:   -r=%t.o,_ZN1B1fEi,p \
+; RUN:   -r=%t.o,_ZN1C1fEi,p \
+; RUN:   -r=%t.o,empty,p \
+; RUN:   -r=%t.o,_ZTV1B, \
+; RUN:   -r=%t.o,_ZTV1C, \
+; RUN:   -r=%t.o,_ZN1A1nEi, \
+; RUN:   -r=%t.o,_ZN1B1fEi, \
+; RUN:   -r=%t.o,_ZN1C1fEi, \
+; RUN:   -r=%t.o,_ZTV1B,px \
+; RUN:   -r=%t.o,_ZTV1C,px 2>&1 | FileCheck %s --check-prefix=NOERROR --allow-empty
+; NOERROR-NOT: failed: inconsistent LTO Unit splitting (recompile with -fsplit-lto-unit)
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-grtev4-linux-gnu"
 
 %struct.A = type { i32 (...)** }
 %struct.B = type { %struct.A }
 %struct.C = type { %struct.A }
 
 @_ZTV1B = constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.B*, i32)* @_ZN1B1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !1