[PATCH] D20290: [ThinLTO] Change ODR resolution and internalization to be index-based

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Fri May 20 08:20:36 PDT 2016


tejohnson added a comment.

In http://reviews.llvm.org/D20290#435241, @joker.eph wrote:

> Cannot link llvm-tblgen after this change, I get some strange errors like:
>
>   ld: warning: direct access in function 'void std::__1::vector<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo> >::__push_back_slow_path<llvm::CGIOperandList::OperandInfo const&>(llvm::CGIOperandList::OperandInfo const&&&)' from file '/tmp/thinlto.o/21.o' to global weak symbol 'std::__1::__split_buffer<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo>&>::~__split_buffer()' from file 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenInstruction.cpp.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
>
>
> and
>
>   ld: internal error: atom not found in symbolIndex(__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcEEEENS_12basic_stringIT_T0_T1_EEPKS6_RKS9_) for architecture x86_64
>
>
> Something is not NFC here...


The weak issue seems like it must be related to the linkonce/weak resolution. Let me see if I can repro this here. Meanwhile I will also fix the issues you found, see responses below.


================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:387
@@ +386,3 @@
+      Linkage = GS->second->linkage();
+    return Linkage == GlobalValue::ExternalLinkage;
+  };
----------------
joker.eph wrote:
> I think one thing is here it should be `return Linkage != GlobalValue::InternalLinkage;`
> 
> (but that's not enough to make the link passing) 
Hmm, yes. I think I was assuming that this would only be called for things that are currently external, but that doesn't seem to be the case. Should actually be:
  return !GlobalValue::isLocalLinkage(Linkage);
to catch private as well.

================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:539
@@ +538,3 @@
+
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+    return ExportList.count(GUID) || GUIDPreservedSymbols.count(GUID);
----------------
joker.eph wrote:
> ModuleIdentifier is ignored here, this is causing an issue for `thinLTOInternalizeAndPromoteInIndex`.
> 
> `thinLTOInternalizeAndPromoteInIndex` should not process the full index but only `DefinedGlobals`
Actually we do want to process the whole index - I can't recall now why I put this here rather than in the caller where it also calls resolveWeakForLinkerInIndex for the entire index. This is bad because multiple threads can be modifying the same index!


http://reviews.llvm.org/D20290





More information about the llvm-commits mailing list