[ThinLTO] Restructure promotion / internalization decisions (NFC)
authorTeresa Johnson <tejohnson@google.com>
Thu, 1 Jun 2023 15:57:35 +0000 (08:57 -0700)
committerTeresa Johnson <tejohnson@google.com>
Thu, 1 Jun 2023 16:08:43 +0000 (09:08 -0700)
Restructures the combined index based promotion and internalization
decision code so that it is a bit easier to follow. This is in
preparation for a bugfix to this code that will modify one part of the
logic.

llvm/lib/LTO/LTO.cpp

index 2f52a20..fa3e060 100644 (file)
@@ -446,39 +446,52 @@ void llvm::thinLTOResolvePrevailingInIndex(
                                  recordNewLinkage, GUIDPreservedSymbols);
 }
 
-static bool isWeakObjectWithRWAccess(GlobalValueSummary *GVS) {
-  if (auto *VarSummary = dyn_cast<GlobalVarSummary>(GVS->getBaseObject()))
-    return !VarSummary->maybeReadOnly() && !VarSummary->maybeWriteOnly() &&
-           (VarSummary->linkage() == GlobalValue::WeakODRLinkage ||
-            VarSummary->linkage() == GlobalValue::LinkOnceODRLinkage);
-  return false;
-}
-
 static void thinLTOInternalizeAndPromoteGUID(
     ValueInfo VI, function_ref<bool(StringRef, ValueInfo)> isExported,
     function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
         isPrevailing) {
   for (auto &S : VI.getSummaryList()) {
+    // First see if we need to promote an internal value because it is not
+    // exported.
     if (isExported(S->modulePath(), VI)) {
       if (GlobalValue::isLocalLinkage(S->linkage()))
         S->setLinkage(GlobalValue::ExternalLinkage);
-    } else if (EnableLTOInternalization &&
-               // Ignore local and appending linkage values since the linker
-               // doesn't resolve them.
-               !GlobalValue::isLocalLinkage(S->linkage()) &&
-               (!GlobalValue::isInterposableLinkage(S->linkage()) ||
-                isPrevailing(VI.getGUID(), S.get())) &&
-               S->linkage() != GlobalValue::AppendingLinkage &&
-               // We can't internalize available_externally globals because this
-               // can break function pointer equality.
-               S->linkage() != GlobalValue::AvailableExternallyLinkage &&
-               // Functions and read-only variables with linkonce_odr and
-               // weak_odr linkage can be internalized. We can't internalize
-               // linkonce_odr and weak_odr variables which are both modified
-               // and read somewhere in the program because reads and writes
-               // will become inconsistent.
-               !isWeakObjectWithRWAccess(S.get()))
-      S->setLinkage(GlobalValue::InternalLinkage);
+      continue;
+    }
+
+    // Otherwise, see if we can internalize.
+    if (!EnableLTOInternalization)
+      continue;
+
+    // Ignore local and appending linkage values since the linker
+    // doesn't resolve them (and there is no need to internalize if this is
+    // already internal).
+    if (GlobalValue::isLocalLinkage(S->linkage()) ||
+        S->linkage() == GlobalValue::AppendingLinkage)
+      continue;
+
+    // We can't internalize available_externally globals because this
+    // can break function pointer equality.
+    if (S->linkage() == GlobalValue::AvailableExternallyLinkage)
+      continue;
+
+    bool IsPrevailing = isPrevailing(VI.getGUID(), S.get());
+
+    if (GlobalValue::isInterposableLinkage(S->linkage()) && !IsPrevailing)
+      continue;
+
+    // Functions and read-only variables with linkonce_odr and weak_odr linkage
+    // can be internalized. We can't internalize linkonce_odr and weak_odr
+    // variables which are both modified and read somewhere in the program
+    // because reads and writes will become inconsistent.
+    auto *VarSummary = dyn_cast<GlobalVarSummary>(S->getBaseObject());
+    if (VarSummary && !VarSummary->maybeReadOnly() &&
+        !VarSummary->maybeWriteOnly() &&
+        (VarSummary->linkage() == GlobalValue::WeakODRLinkage ||
+         VarSummary->linkage() == GlobalValue::LinkOnceODRLinkage))
+      continue;
+
+    S->setLinkage(GlobalValue::InternalLinkage);
   }
 }