This is an archive of the discontinued LLVM Phabricator instance.

Differential D101419

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

Authored by dexonsmith on Apr 27 2021, 6:13 PM.

Details

Reviewers
mehdi_amini
steven_wu
Commits
rG9131a078901b: Linker: Avoid scheduling the link of a global value twice due to an alias
Summary

3d4f3a0da90bd1a34ff3ef4c3b99c1dbcca2e6cb (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.

Diff Detail

Event Timeline

dexonsmith created this revision.Apr 27 2021, 6:13 PM
Herald added a subscriber: hiraditya. · View Herald TranscriptApr 27 2021, 6:13 PM
dexonsmith requested review of this revision.Apr 27 2021, 6:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2021, 6:13 PM
dexonsmith mentioned this in D99546: [llvm][ValueMap] Share the value map between MappingContext.Apr 27 2021, 6:14 PM
dexonsmith edited the summary of this revision. (Show Details)Apr 27 2021, 6:18 PM
mehdi_amini accepted this revision.Apr 27 2021, 6:33 PM
This revision is now accepted and ready to land.Apr 27 2021, 6:33 PM
steven_wu accepted this revision.Apr 27 2021, 6:37 PM
Harbormaster completed remote builds in B101310: Diff 341046.Apr 27 2021, 8:56 PM
This revision was landed with ongoing or failed builds.Apr 28 2021, 1:22 PM
Closed by commit rG9131a078901b: Linker: Avoid scheduling the link of a global value twice due to an alias (authored by dexonsmith). · Explain Why
This revision was automatically updated to reflect the committed changes.
dexonsmith added a commit: rG9131a078901b: Linker: Avoid scheduling the link of a global value twice due to an alias.

Revision Contents

PathSize
llvm/
lib/
Linker/
15 lines
test/
Linker/
12 lines

Diff 341046

llvm/lib/Linker/IRMover.cpp

Show First 20 Lines • Show All 604 Lines • ▼ Show 20 Lines if (auto *F = dyn_cast<Function>(New)) {
if (V->hasInitializer() || V->hasAppendingLinkage()) if (V->hasInitializer() || V->hasAppendingLinkage())
return New; return New;
} else { } else {
auto *IS = cast<GlobalIndirectSymbol>(New); auto *IS = cast<GlobalIndirectSymbol>(New);
if (IS->getIndirectSymbol()) if (IS->getIndirectSymbol())
return New; return New;
} }
// When linking a global for an indirect symbol, it will always be linked. // If the global is being linked for an indirect symbol, it may have already
// However we need to check if it was not already scheduled to satisfy a // been scheduled to satisfy a regular symbol. Similarly, a global being linked
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  // been scheduled to satisfy a regular symbol. Similarly, a global being linked
-  // for a regular symbol may have already been scheduled for an indirect
+  // been scheduled to satisfy a regular symbol. Similarly, a global being
+  // linked for a regular symbol may have already been scheduled for an indirect
Lint: Pre-merge checks: clang-format: please reformat the code ``` - // been scheduled to satisfy a regular symbol.
// reference from a regular global value initializer. We know if it has been // for a regular symbol may have already been scheduled for an indirect
// schedule if the "New" GlobalValue that is mapped here for the indirect // symbol. Check for these cases by looking in the other value map and
// symbol is the same as the one already mapped. If there is an entry in the // confirming the same value has been scheduled. If there is an entry in the
// ValueMap but the value is different, it means that the value already had a // ValueMap but the value is different, it means that the value already had a
// definition in the destination module (linkonce for instance), but we need a // definition in the destination module (linkonce for instance), but we need a
// new definition for the indirect symbol ("New" will be different. // new definition for the indirect symbol ("New" will be different).
if (ForIndirectSymbol && ValueMap.lookup(SGV) == New) if ((ForIndirectSymbol && ValueMap.lookup(SGV) == New) ||
(!ForIndirectSymbol && IndirectSymbolValueMap.lookup(SGV) == New))
return New; return New;
if (ForIndirectSymbol || shouldLink(New, *SGV)) if (ForIndirectSymbol || shouldLink(New, *SGV))
setError(linkGlobalValueBody(*New, *SGV)); setError(linkGlobalValueBody(*New, *SGV));
return New; return New;
} }
▲ Show 20 Lines • Show All 1,006 Lines • Show Last 20 Lines

llvm/test/Linker/alias-4.ll

  • This file was added.
; 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
@gv0 = constant i64* @alias
@gv1 = constant i64 ptrtoint (i64* @gv1 to i64)
@alias = alias i64, i64* @gv1