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


Hi Mehdi,

test/ThinLTO/X86/cache.ll now fails on my build system, which uses:

$ /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.

 -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


More information about the llvm-commits mailing list