LTO: Include live bit in ThinLTO cache key.

[pcc]

Summary

As of r323633, this bit started controlling whether symbol definitions
appear in object files, and is also became sensitive to the prevailing
bit, so it needs to be included in the key.

[dberris]

LGTM

[tejohnson]

Wondering whether there is a better way of proactively catching these issues - I assume it is currently ultimately detected through either linker failures or even worse, runtime failures. The only fully automated idea I have come up with is to have a debugging mode where we would force a native object to be produced even for cache hits, and would assert that the new and old objects are identical. Since that is expensive, the question would be how to run that broadly enough to catch bugs like this quickly, before it reaches the wild. Since the LLVM revision is used in the key, it would have to be using the same clang to build something large enough but at either different source revisions or with different enough options.

Any other possibilities?

[pcc]

In D43109#1003417, @tejohnson wrote:

Wondering whether there is a better way of proactively catching these issues - I assume it is currently ultimately detected through either linker failures or even worse, runtime failures. The only fully automated idea I have come up with is to have a debugging mode where we would force a native object to be produced even for cache hits, and would assert that the new and old objects are identical. Since that is expensive, the question would be how to run that broadly enough to catch bugs like this quickly, before it reaches the wild. Since the LLVM revision is used in the key, it would have to be using the same clang to build something large enough but at either different source revisions or with different enough options.

Any other possibilities?

Generally in Chromium these types of bugs have resulted in cache entries being incorrectly shared between targets (i.e. executables or DSOs, not target architectures). In the past I have tried to identify them by giving each target its own cache directory, building all the targets in the project and using "diff" to identify files with identical keys but different content. I would imagine that that could be relatively easily automated given a project willing to support that configuration (perhaps Chromium would be a good candidate). The advantages are that it would not require compiler support and it would allow the names of the mismatching targets to be easily reported. The disadvantage is that it would require more disk space (which could be a problem for projects such as Chromium with a large number of targets) but perhaps it wouldn't be a problem in practice if we disable debug info.

llvm/lib/LTO/LTO.cpp

auto AddUsedCfiGlobal = [&](GlobalValue::GUID ValueGUID) {
     if (CfiFunctionDefs.count(ValueGUID))
       UsedCfiDefs.insert(ValueGUID);
     if (CfiFunctionDecls.count(ValueGUID))
       UsedCfiDecls.insert(ValueGUID);
   };
 
   auto AddUsedThings = [&](GlobalValueSummary *GS) {
     if (!GS) return;
+    AddUnsigned(GS->isLive());
     for (const ValueInfo &VI : GS->refs()) {
       AddUnsigned(VI.isDSOLocal());
       AddUsedCfiGlobal(VI.getGUID());
     }
     if (auto *FS = dyn_cast<FunctionSummary>(GS)) {
       for (auto &TT : FS->type_tests())
         UsedTypeIds.insert(TT);
       for (auto &TT : FS->type_test_assume_vcalls())

llvm/test/LTO/Resolution/X86/cache-prevailing.ll

+; Tests whether the cache is sensitive to the prevailing bit.
+; RUN: rm -rf %t.cache
+; RUN: opt -module-hash -module-summary -o %t.bc %s
+; RUN: llvm-lto2 run -o %t.o %t.bc -cache-dir %t.cache \
+; RUN:   -r %t.bc,foo,p -r %t.bc,bar,px
+; RUN: llvm-lto2 run -o %t.o %t.bc -cache-dir %t.cache \
+; RUN:   -r %t.bc,foo, -r %t.bc,bar,px
+; RUN: ls %t.cache | count 2
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.11.0"
+
+@foo = linkonce_odr constant i32 1, comdat
+$foo = comdat any
+
+define i32* @bar() {
+  ret i32* @foo
+}