[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:29:15 PDT 2016


----- Original Message -----
> From: "Hal Finkel via llvm-commits" <llvm-commits at lists.llvm.org>
> To: "Mehdi Amini" <mehdi.amini at apple.com>
> Cc: "llvm-commits" <llvm-commits at lists.llvm.org>
> Sent: Saturday, October 8, 2016 6:06:31 PM
> Subject: Re: [llvm] r283655 - ThinLTO: don't perform incremental LTO on module without a hash
> 
> ----- 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...

That fixed it, thanks again!

 -Hal

> 
>  -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
> _______________________________________________
> 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


More information about the llvm-commits mailing list