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

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 9 07:49:29 PDT 2016


On Sat, Oct 8, 2016 at 4:00 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

> 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
>
> >
> > $ /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/t
> o/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?
>

Yes, almost certainly there are several. I haven't set up any bots myself,
but a quick look at the git log of test/tools/gold/ shows that I have made
several test fixes due to bot failures. Some of the bots are listed in
those commit messages.

Maybe the bots were already failing due to something else and so you didn't
get any notifications?

Teresa


>
>> 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/ThinL
> TOCodeGenerator.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
>
>


-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161009/096aeb96/attachment.html>


More information about the llvm-commits mailing list