Skip to content

Commit 02054d3

Browse files
committed
[LTO] Simplify internalize logic. NFC
D151965 removed incorrect internalization for {linkonce,weak}{,_odr} when the prevailing copy is in native code. The multiple conditions are based on negative conditions, which can be simplified to be based on positive cases: * an object with an external linkage (must be prevailing) can be internalized * a prevailing object with a {linkonce,weak}{,_odr} or common linkage can be internalized. Further, the lengthy comment is a bit misleading, as it doesn't say that objects with an external/linkonce/weak linkage can be internalized. Clarify it. Reviewed By: tejohnson Differential Revision: https://reviews.llvm.org/D158949
1 parent 5fe6f56 commit 02054d3

File tree

1 file changed

+14
-27
lines changed

1 file changed

+14
-27
lines changed

llvm/lib/LTO/LTO.cpp

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -468,24 +468,13 @@ static void thinLTOInternalizeAndPromoteGUID(
468468
if (!EnableLTOInternalization)
469469
continue;
470470

471-
// Ignore local and appending linkage values since the linker
472-
// doesn't resolve them (and there is no need to internalize if this is
473-
// already internal).
474-
if (GlobalValue::isLocalLinkage(S->linkage()) ||
475-
S->linkage() == GlobalValue::AppendingLinkage)
476-
continue;
477-
478-
// We can't internalize available_externally globals because this
479-
// can break function pointer equality.
480-
if (S->linkage() == GlobalValue::AvailableExternallyLinkage)
481-
continue;
482-
483-
bool IsPrevailing = isPrevailing(VI.getGUID(), S.get());
484-
485-
if (GlobalValue::isInterposableLinkage(S->linkage()) && !IsPrevailing)
471+
// Non-exported values with external linkage can be internalized.
472+
if (GlobalValue::isExternalLinkage(S->linkage())) {
473+
S->setLinkage(GlobalValue::InternalLinkage);
486474
continue;
475+
}
487476

488-
// Non-exported functions and variables with linkonce_odr or weak_odr
477+
// Non-exported function and variable definitions with a weak-for-linker
489478
// linkage can be internalized in certain cases. The minimum legality
490479
// requirements would be that they are not address taken to ensure that we
491480
// don't break pointer equality checks, and that variables are either read-
@@ -494,7 +483,7 @@ static void thinLTOInternalizeAndPromoteGUID(
494483
// (which is how this is guaranteed for variables, when analyzing whether
495484
// they are read or write-only).
496485
//
497-
// However, we only get to this code for weak/linkonce ODR values in one of
486+
// However, we only get to this code for weak-for-linkage values in one of
498487
// two cases:
499488
// 1) The prevailing copy is not in IR (it is in native code).
500489
// 2) The prevailing copy in IR is not exported from its module.
@@ -506,10 +495,10 @@ static void thinLTOInternalizeAndPromoteGUID(
506495
// duplicate linkonce_odr copies as exported via the tool, so we need
507496
// to handle that case below by checking the number of copies.
508497
//
509-
// Generally, we only want to internalize a linkonce/weak ODR value in case
498+
// Generally, we only want to internalize a weak-for-linker value in case
510499
// 2, because in case 1 we cannot see how the value is used to know if it
511500
// is read or write-only. We also don't want to bloat the binary with
512-
// multiple internalized copies of non-prevailing linkonce_odr functions.
501+
// multiple internalized copies of non-prevailing linkonce/weak functions.
513502
// Note if we don't internalize, we will convert non-prevailing copies to
514503
// available_externally anyway, so that we drop them after inlining. The
515504
// only reason to internalize such a function is if we indeed have a single
@@ -520,18 +509,16 @@ static void thinLTOInternalizeAndPromoteGUID(
520509
// already perform this elsewhere in the ThinLTO backend handling for
521510
// read or write-only variables (processGlobalForThinLTO).
522511
//
523-
// Therefore, only internalize linkonce/weak ODR if there is a single copy,
524-
// that is prevailing in this IR module. We can do so aggressively, without
512+
// Therefore, only internalize linkonce/weak if there is a single copy, that
513+
// is prevailing in this IR module. We can do so aggressively, without
525514
// requiring the address to be insignificant, or that a variable be read or
526515
// write-only.
527-
if ((S->linkage() == GlobalValue::WeakODRLinkage ||
528-
S->linkage() == GlobalValue::LinkOnceODRLinkage) &&
529-
// We can have only one copy in ThinLTO that isn't prevailing, if the
530-
// prevailing copy is in a native object.
531-
(!IsPrevailing || ExternallyVisibleCopies > 1))
516+
if (!GlobalValue::isWeakForLinker(S->linkage()) ||
517+
GlobalValue::isExternalWeakLinkage(S->linkage()))
532518
continue;
533519

534-
S->setLinkage(GlobalValue::InternalLinkage);
520+
if (isPrevailing(VI.getGUID(), S.get()) && ExternallyVisibleCopies == 1)
521+
S->setLinkage(GlobalValue::InternalLinkage);
535522
}
536523
}
537524

0 commit comments

Comments
 (0)