[llvm] 9131a07 - Linker: Avoid scheduling the link of a global value twice due to an alias

[Duncan P. N. Exon Smith via llvm-commits]

Author: Duncan P. N. Exon Smith
Date: 2021-04-28T13:22:10-07:00
New Revision: 9131a078901b00e68248a27a4f8c4b11bb1db1ae

URL: https://github.com/llvm/llvm-project/commit/9131a078901b00e68248a27a4f8c4b11bb1db1ae
DIFF: https://github.com/llvm/llvm-project/commit/9131a078901b00e68248a27a4f8c4b11bb1db1ae.diff

LOG: Linker: Avoid scheduling the link of a global value twice due to an alias

3d4f3a0da90bd1a3 (https://reviews.llvm.org/D20586) avoided rescheduling
a global value that was materialized first through a regular value, and
then again through an alias. This commit catches the dual, avoiding
rescheduling when the global value is first materialized through an
alias.

Differential Revision: https://reviews.llvm.org/D101419
Radar-Id: rdar://75752728

Added: 
    llvm/test/Linker/alias-4.ll

Modified: 
    llvm/lib/Linker/IRMover.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 9489f9655469c..e67a5b42dc4c0 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -610,15 +610,16 @@ Value *IRLinker::materialize(Value *V, bool ForIndirectSymbol) {
       return New;
   }
 
-  // When linking a global for an indirect symbol, it will always be linked.
-  // However we need to check if it was not already scheduled to satisfy a
-  // reference from a regular global value initializer. We know if it has been
-  // schedule if the "New" GlobalValue that is mapped here for the indirect
-  // symbol is the same as the one already mapped. If there is an entry in the
+  // If the global is being linked for an indirect symbol, it may have already
+  // been scheduled to satisfy a regular symbol. Similarly, a global being linked
+  // for a regular symbol may have already been scheduled for an indirect
+  // symbol. Check for these cases by looking in the other value map and
+  // confirming the same value has been scheduled.  If there is an entry in the
   // ValueMap but the value is 
diff erent, it means that the value already had a
   // definition in the destination module (linkonce for instance), but we need a
-  // new definition for the indirect symbol ("New" will be 
diff erent.
-  if (ForIndirectSymbol && ValueMap.lookup(SGV) == New)
+  // new definition for the indirect symbol ("New" will be 
diff erent).
+  if ((ForIndirectSymbol && ValueMap.lookup(SGV) == New) ||
+      (!ForIndirectSymbol && IndirectSymbolValueMap.lookup(SGV) == New))
     return New;
 
   if (ForIndirectSymbol || shouldLink(New, *SGV))

diff  --git a/llvm/test/Linker/alias-4.ll b/llvm/test/Linker/alias-4.ll
new file mode 100644
index 0000000000000..06de4b2836279
--- /dev/null
+++ b/llvm/test/Linker/alias-4.ll
@@ -0,0 +1,12 @@
+; RUN: llvm-link %s -S -o - | FileCheck %s
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.9"
+
+; CHECK-DAG: @gv0 = constant i64* @alias
+; CHECK-DAG: @gv1 = constant i64 ptrtoint (i64* @gv1 to i64)
+; CHECK-DAG: @alias = alias i64, i64* @gv1
+
+ at gv0 = constant i64* @alias
+ at gv1 = constant i64 ptrtoint (i64* @gv1 to i64)
+
+ at alias = alias i64, i64* @gv1