[llvm] r283655 - ThinLTO: don't perform incremental LTO on module without a hash
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 8 16:00:01 PDT 2016
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/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
More information about the llvm-commits
mailing list