[PATCH] D104986: [ThinLTO] Respect ClearDSOLocalOnDeclarations for unimported functions

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added a reviewer: tejohnson.
Herald added subscribers: ormris, steven_wu, hiraditya, inglorion.
Herald added a reviewer: bollu.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

D74751 <https://reviews.llvm.org/D74751> added `ClearDSOLocalOnDeclarations` and dropped dso_local for
isDeclarationForLinker `GlobalValue`s. It missed a case for unimported functions
(`doImportAsDefinition` is false) when `isPerformingImport` is true.
This can lead to a linker error for a default visibility symbol in
`ld.lld -shared`.

The root cause is: in `FunctionImportGlobalProcessing::getLinkage`,
`doImportAsDefinition` should be `isDeclarationForLinker` instead, i.e.
when `isPerformingImport`, an unimported `define dso_local @foo` should
be treated as an available_externally function, instead of an
ExternalLinkage definition. The issue has been benign, until we start to
consider ClearDSOLocalOnDeclarations. (`doImportAsDefinition` logic has
poor test coverage. It mostly doesn't matter because in IRMover.cpp,
linkages for non-extern_weak declarations will be ignored anyway.)

This patch fixes a `LLVMPolly.so` link error using a trunk clang -DLLVM_ENABLE_LTO=Thin.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104986

Files:
  llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
  llvm/test/ThinLTO/X86/import-dsolocal.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104986.354716.patch
Type: text/x-patch
Size: 5841 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210626/a963961c/attachment.bin>