[PATCH] D35064: [lib/LTO] Fix the interaction between linker redefined symbols and ThinLTO

[Davide Italiano via Phabricator via llvm-commits]

davide created this revision.
Herald added subscribers: inglorion, mehdi_amini.

This is the same of https://reviews.llvm.org/rL304719 but for ThinLTO.
The substantial difference is that in this case we don't have whole visibility, just the summary.
In the LTO case, when we got the resolution for the input file we could just see if the linker told us whether a symbol was linker redefined (using --wrap or --defsym) and switch the linkage directly for the GV.

Here, we have the summary. So, we record that the linkage changed from <whatever it was> to $weakany to prevent IPOs across this symbol boundaries and actually just switch the linkage at `FunctionImport` time.

This patch should also fix the lld bits (as all the scaffolding for communicating if a symbol is linker redefined should be there & should be the same), but I'll make sure to add some tests there as well.

Fixes https://bugs.llvm.org/show_bug.cgi?id=33192


https://reviews.llvm.org/D35064

Files:
  lib/LTO/LTO.cpp
  lib/Transforms/IPO/FunctionImport.cpp
  test/LTO/Resolution/X86/linker-redef-thin.ll


Index: test/LTO/Resolution/X86/linker-redef-thin.ll
===================================================================
--- /dev/null
+++ test/LTO/Resolution/X86/linker-redef-thin.ll
@@ -0,0 +1,16 @@
+; RUN: opt -module-summmary %s -o %t.o
+; RUN: llvm-lto2 run -o %t1.o %t.o -r %t.o,bar,pr
+; RUN: llvm-readobj -t %t1.o.0 | FileCheck %s
+
+; CHECK: Name: bar
+; CHECK-NEXT: Value:
+; CHECK-NEXT: Size:
+; CHECK-NEXT: Binding: Weak
+; CHECK-NEXT: Type: Function
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @bar() {
+  ret void
+}
Index: lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- lib/Transforms/IPO/FunctionImport.cpp
+++ lib/Transforms/IPO/FunctionImport.cpp
@@ -537,7 +537,11 @@
   };
 
   auto updateLinkage = [&](GlobalValue &GV) {
-    if (!GlobalValue::isWeakForLinker(GV.getLinkage()))
+    // Only rename symbols that are weak for the linker, i.e.
+    // have linkonce or linkonce_odr linkage or have been explicitly
+    // redefined at link time, via, e.g. --wrap or -defsym (in ELF).
+    if (GV.getLinkage() == GlobalValue::AppendingLinkage ||
+        GV.getLinkage() == GlobalValue::ExternalLinkage)
       return;
     // See if the global summary analysis computed a new resolved linkage.
     const auto &GS = DefinedGlobals.find(GV.getGUID());
Index: lib/LTO/LTO.cpp
===================================================================
--- lib/LTO/LTO.cpp
+++ lib/LTO/LTO.cpp
@@ -665,6 +665,15 @@
         auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
             Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
         ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
+
+        // For linker redefined symbols (via --wrap or --defsym) we want to
+        // switch the linkage to `weak` to prevent IPOs from happening.
+        // Find the summary in the module for this very GV and record the new
+        // linkage so that we can switch it when we import the GV.
+        auto S = ThinLTO.CombinedIndex.findSummaryInModule(
+            GUID, BM.getModuleIdentifier());
+        if (S && Res.LinkerRedefined)
+          S->setLinkage(GlobalValue::WeakAnyLinkage);
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35064.105448.patch
Type: text/x-patch
Size: 2298 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170706/b0761812/attachment.bin>