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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/FunctionImportUtils.cpp:130
+    // to an imported externally visible global value.
+    if (!SGV->isDeclarationForLinker() && !isa<GlobalAlias>(SGV))
       return GlobalValue::AvailableExternallyLinkage;
----------------
I don't follow what is happening with and without this change. If we aren't planning to import something as a definition, why do we need to mark it available_externally in the source module? How does this change result in dropping the dso local flag?

>From your summary:
> i.e. when isPerformingImport, an unimported define dso_local @foo should be treated as an available_externally function, instead of an ExternalLinkage definition. 

It should eventually end up as an external declaration. Is that not happening? Or is the available_externally marking just a temporary adjustment to trigger subsequent dso_local clearing?


================
Comment at: llvm/test/ThinLTO/X86/import-dsolocal.ll:63
+
+define weak_odr dso_local void @linkonceodr() {
+  call i32 @linkonceodr_aux(i32* @a, i32** @b)
----------------
Should this have linkonce_odr linkage?


================
Comment at: llvm/test/ThinLTO/X86/import-dsolocal.ll:68
+
+define weak_odr i32 @linkonceodr_aux(i32* %a, i32** %b) {
+  %p = load i32*, i32** %b, align 8
----------------
Ditto


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104986/new/

https://reviews.llvm.org/D104986