Revert D135427 "[LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally"

This reverts commit 8ef3fd8d59ba0100bc6e83350ab1e978536aa531.

I mentioned that GlobalAlias was not handled. It turns out GlobalAlias has to be handled in the same patch (as opposed to in a follow-up),
as otherwise clang codegen of C5/D5 constructor/destructor would regress (https://reviews.llvm.org/D135427#3869003).

2 files changed, 6 insertions, 21 deletions

diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index dc28b68..286d3ca 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -712,11 +712,11 @@ handleNonPrevailingComdat(GlobalValue &GV,
   if (!NonPrevailingComdats.count(C))
     return;
 
-  // Additionally need to drop all global values from the comdat to
-  // available_externally, to satisfy the COMDAT requirement that all members
-  // are discarded as a unit. The non-local linkage global values avoid
-  // duplicate definition linker errors.
-  GV.setLinkage(GlobalValue::AvailableExternallyLinkage);
+  // Additionally need to drop externally visible global values from the comdat
+  // to available_externally, so that there aren't multiply defined linker
+  // errors.
+  if (!GV.hasLocalLinkage())
+    GV.setLinkage(GlobalValue::AvailableExternallyLinkage);
 
   if (auto GO = dyn_cast<GlobalObject>(&GV))
     GO->setComdat(nullptr);
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index c5b2135..b589ec7 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1051,7 +1051,6 @@ bool llvm::convertToDeclaration(GlobalValue &GV) {
 void llvm::thinLTOFinalizeInModule(Module &TheModule,
                                    const GVSummaryMapTy &DefinedGlobals,
                                    bool PropagateAttrs) {
-  DenseSet<Comdat *> NonPrevailingComdats;
   auto FinalizeInModule = [&](GlobalValue &GV, bool Propagate = false) {
     // See if the global summary analysis computed a new resolved linkage.
     const auto &GS = DefinedGlobals.find(GV.getGUID());
@@ -1129,10 +1128,8 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
     // as this is a declaration for the linker, and will be dropped eventually.
     // It is illegal for comdats to contain declarations.
     auto *GO = dyn_cast_or_null<GlobalObject>(&GV);
-    if (GO && GO->isDeclarationForLinker() && GO->hasComdat()) {
-      NonPrevailingComdats.insert(GO->getComdat());
+    if (GO && GO->isDeclarationForLinker() && GO->hasComdat())
       GO->setComdat(nullptr);
-    }
   };
 
   // Process functions and global now
@@ -1142,18 +1139,6 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
     FinalizeInModule(GV);
   for (auto &GV : TheModule.aliases())
     FinalizeInModule(GV);
-
-  // For a non-prevailing comdat, all its members must be available_externally.
-  // FinalizeInModule has handled non-local-linkage GlobalValues. Here we handle
-  // local linkage GlobalValues.
-  if (NonPrevailingComdats.empty())
-    return;
-  for (auto &GO : TheModule.global_objects()) {
-    if (auto *C = GO.getComdat(); C && NonPrevailingComdats.count(C)) {
-      GO.setComdat(nullptr);
-      GO.setLinkage(GlobalValue::AvailableExternallyLinkage);
-    }
-  }
 }
 
 /// Run internalization on \p TheModule based on symmary analysis.