[llvm] r283655 - ThinLTO: don't perform incremental LTO on module without a hash

Hal Finkel via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 8 16:06:31 PDT 2016


----- Original Message -----
> From: "Mehdi Amini" <mehdi.amini at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "llvm-commits" <llvm-commits at lists.llvm.org>, "Teresa Johnson" <tejohnson at google.com>
> Sent: Saturday, October 8, 2016 6:00:01 PM
> Subject: Re: [llvm] r283655 - ThinLTO: don't perform incremental LTO on module without a hash
> 
> Hi Hal!
> 
> 
> > On Oct 8, 2016, at 3:51 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > 
> > Hi Mehdi,
> > 
> > test/ThinLTO/X86/cache.ll now fails on my build system, which uses:
> 
> Looking at the trace below, it looks like
> test/tools/gold/X86/cache.ll

Indeed, yes! Thanks, for the quick update, checking now...

 -Hal

> 
> > 
> > $ /usr/bin/ld.gold --version
> > GNU gold (version 2.23.52.0.1-55.el7 20130226) 1.11
> > 
> > And runs these commands:
> > 
> > /path/to/build/llvm-stage1/./bin/opt -module-summary
> > /path/to/src/llvm/test/tools/gold/X86/cache.ll -o
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp.o
> > /path/to/build/llvm-stage1/./bin/opt -module-summary
> > /path/to/src/llvm/test/tools/gold/X86/Inputs/cache.ll -o
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp2.o
> > rm -Rf
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp.cache
> > && mkdir
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp.cache
> > /usr/bin/ld.gold -m elf_x86_64 -plugin
> > /path/to/build/llvm-stage1/./lib/LLVMgold.so
> >      --plugin-opt=thinlto
> >      --plugin-opt=cache-dir=/path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp.cache
> >      -o
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp3.o
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp2.o
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp.o
> > ls
> > /path/to/build/llvm-stage1/test/tools/gold/X86/Output/cache.ll.tmp.cache
> > | /path/to/build/llvm-stage1/./bin/count 2
> > 
> > All of the commands except the final one, specifically including
> > the ld.gold command, succeed. The Output/cache.ll.tmp3.o output is
> > indeed created. However, the cache.ll.tmp.cache is empty, and so
> > the final count check fails.
> 
> Makes sense, this is the same test as test/ThinLTO/X86/cache.ll ; I
> updated it the same way in r283681.
> Let me know if it fixes it for you!
> 
> +CC Teresa: Don’t we have a bot that runs with Gold?
> 
>> Mehdi
> 
> 
> 
> > 
> > -Hal
> > 
> > ----- Original Message -----
> >> From: "Mehdi Amini via llvm-commits" <llvm-commits at lists.llvm.org>
> >> To: llvm-commits at lists.llvm.org
> >> Sent: Friday, October 7, 2016 11:44:24 PM
> >> Subject: [llvm] r283655 - ThinLTO: don't perform incremental LTO
> >> on module without a hash
> >> 
> >> Author: mehdi_amini
> >> Date: Fri Oct  7 23:44:23 2016
> >> New Revision: 283655
> >> 
> >> URL: http://llvm.org/viewvc/llvm-project?rev=283655&view=rev
> >> Log:
> >> ThinLTO: don't perform incremental LTO on module without a hash
> >> 
> >> Clang always emit a hash for ThinLTO, but as other frontend are
> >> starting to use ThinLTO, this could be a serious bug.
> >> 
> >> Differential Revision: https://reviews.llvm.org/D25379
> >> 
> >> Modified:
> >>    llvm/trunk/lib/LTO/LTO.cpp
> >>    llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
> >>    llvm/trunk/test/ThinLTO/X86/cache.ll
> >>    llvm/trunk/test/ThinLTO/X86/empty_module_with_cache.ll
> >> 
> >> Modified: llvm/trunk/lib/LTO/LTO.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTO.cpp?rev=283655&r1=283654&r2=283655&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/lib/LTO/LTO.cpp (original)
> >> +++ llvm/trunk/lib/LTO/LTO.cpp Fri Oct  7 23:44:23 2016
> >> @@ -542,8 +542,12 @@ public:
> >>     };
> >> 
> >>     auto ModuleID = MBRef.getBufferIdentifier();
> >> -    if (!Cache || !CombinedIndex.modulePaths().count(ModuleID))
> >> -      // Cache disabled or no entry for this module in the
> >> combined
> >> index
> >> +
> >> +    if (!Cache || !CombinedIndex.modulePaths().count(ModuleID) ||
> >> +        all_of(CombinedIndex.getModuleHash(ModuleID),
> >> +               [](uint32_t V) { return V == 0; }))
> >> +      // Cache disabled or no entry for this module in the
> >> combined
> >> index or
> >> +      // no module hash.
> >>       return RunThinBackend(AddStream);
> >> 
> >>     SmallString<40> Key;
> >> 
> >> Modified: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp?rev=283655&r1=283654&r2=283655&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp (original)
> >> +++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp Fri Oct  7
> >> 23:44:23
> >> 2016
> >> @@ -243,6 +243,13 @@ public:
> >>     // export list, the hash for every single module in the import
> >>     list, the
> >>     // list of ResolvedODR for the module, and the list of
> >>     preserved
> >>     symbols.
> >> 
> >> +    // Include the hash for the current module
> >> +    auto ModHash = Index.getModuleHash(ModuleID);
> >> +
> >> +    if (all_of(ModHash, [](uint32_t V) { return V == 0; }))
> >> +      // No hash entry, no caching!
> >> +      return;
> >> +
> >>     SHA1 Hasher;
> >> 
> >>     // Start with the compiler revision
> >> @@ -251,8 +258,6 @@ public:
> >>     Hasher.update(LLVM_REVISION);
> >> #endif
> >> 
> >> -    // Include the hash for the current module
> >> -    auto ModHash = Index.getModuleHash(ModuleID);
> >>     Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0],
> >>     sizeof(ModHash)));
> >>     for (auto F : ExportList)
> >>       // The export list can impact the internalization, be
> >>       conservative here
> >> 
> >> Modified: llvm/trunk/test/ThinLTO/X86/cache.ll
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/cache.ll?rev=283655&r1=283654&r2=283655&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/test/ThinLTO/X86/cache.ll (original)
> >> +++ llvm/trunk/test/ThinLTO/X86/cache.ll Fri Oct  7 23:44:23 2016
> >> @@ -1,6 +1,28 @@
> >> +; Verify first that *without* hash, we don't use the cache.
> >> +
> >> ; RUN: opt -module-summary %s -o %t.bc
> >> ; RUN: opt -module-summary %p/Inputs/cache.ll -o %t2.bc
> >> 
> >> +; Verify that enabling caching is ignoring module without hash
> >> +; RUN: rm -Rf %t.cache && mkdir %t.cache
> >> +; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc
> >> %t2.bc  %t.bc -thinlto-cache-dir %t.cache
> >> +; RUN: ls %t.cache/llvmcache.timestamp
> >> +; RUN: ls %t.cache | count 1
> >> +
> >> +; Verify that enabling caching is ignoring module without hash
> >> with
> >> llvm-lto2
> >> +; RUN: rm -Rf %t.cache && mkdir %t.cache
> >> +; RUN: llvm-lto2 -o %t.o %t2.bc  %t.bc -cache-dir %t.cache \
> >> +; RUN:  -r=%t2.bc,_main,plx \
> >> +; RUN:  -r=%t2.bc,_globalfunc,lx \
> >> +; RUN:  -r=%t.bc,_globalfunc,plx
> >> +; RUN: ls %t.cache | count 0
> >> +
> >> +
> >> +; Repeat again, *with* hash this time.
> >> +
> >> +; RUN: opt -module-hash -module-summary %s -o %t.bc
> >> +; RUN: opt -module-hash -module-summary %p/Inputs/cache.ll -o
> >> %t2.bc
> >> +
> >> ; Verify that enabling caching is working
> >> ; RUN: rm -Rf %t.cache && mkdir %t.cache
> >> ; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc
> >> %t2.bc  %t.bc -thinlto-cache-dir %t.cache
> >> 
> >> Modified: llvm/trunk/test/ThinLTO/X86/empty_module_with_cache.ll
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/empty_module_with_cache.ll?rev=283655&r1=283654&r2=283655&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/test/ThinLTO/X86/empty_module_with_cache.ll
> >> (original)
> >> +++ llvm/trunk/test/ThinLTO/X86/empty_module_with_cache.ll Fri Oct
> >>  7
> >> 23:44:23 2016
> >> @@ -22,13 +22,13 @@
> >> ; RUN: rm -Rf %t.cache && mkdir %t.cache
> >> ; RUN: llvm-lto -thinlto-action=run %t2.bc  %t.bc
> >> -exported-symbol=main -thinlto-cache-dir %t.cache
> >> ; RUN: ls %t.cache/llvmcache.timestamp
> >> -; RUN: ls %t.cache | count 2
> >> +; RUN: ls %t.cache | count 1
> >> 
> >> ; Verify that caching is disabled for module without hash, with
> >> llvm-lto2
> >> ; RUN: rm -Rf %t.cache && mkdir %t.cache
> >> ; RUN: llvm-lto2 -o %t.o %t2.bc  %t.bc -cache-dir %t.cache \
> >> ; RUN:  -r=%t2.bc,_main,plx
> >> -; RUN: ls %t.cache | count 1
> >> +; RUN: ls %t.cache | count 0
> >> 
> >> 
> >> target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
> >> 
> >> 
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >> 
> > 
> > --
> > Hal Finkel
> > Lead, Compiler Technology and Programming Languages
> > Leadership Computing Facility
> > Argonne National Laboratory
> 
> 

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory


More information about the llvm-commits mailing list