Merge pull request swiftlang#33454 from compnerd/comdat-cleanup

IRGen: be less aggressive about applying COMDAT

Author: compnerd

include/swift/IRGen/Linking.h

@@ -1231,7 +1231,7 @@ class ApplyIRLinkage {
   IRLinkage IRL;
 public:
   ApplyIRLinkage(IRLinkage IRL) : IRL(IRL) {}
-  void to(llvm::GlobalValue *GV) const {
+  void to(llvm::GlobalValue *GV, bool definition = true) const {
     llvm::Module *M = GV->getParent();
     const llvm::Triple Triple(M->getTargetTriple());
 
@@ -1244,11 +1244,14 @@ class ApplyIRLinkage {
     if (Triple.isOSBinFormatELF())
       return;
 
-    if (IRL.Linkage == llvm::GlobalValue::LinkOnceODRLinkage ||
-        IRL.Linkage == llvm::GlobalValue::WeakODRLinkage)
-      if (Triple.supportsCOMDAT())
-        if (llvm::GlobalObject *GO = dyn_cast<llvm::GlobalObject>(GV))
-          GO->setComdat(M->getOrInsertComdat(GV->getName()));
+    // COMDATs cannot be applied to declarations.  If we have a definition,
+    // apply the COMDAT.
+    if (definition)
+      if (IRL.Linkage == llvm::GlobalValue::LinkOnceODRLinkage ||
+          IRL.Linkage == llvm::GlobalValue::WeakODRLinkage)
+        if (Triple.supportsCOMDAT())
+          if (llvm::GlobalObject *GO = dyn_cast<llvm::GlobalObject>(GV))
+            GO->setComdat(M->getOrInsertComdat(GV->getName()));
   }
 };
 

lib/IRGen/GenDecl.cpp

@@ -2024,8 +2024,10 @@ llvm::Function *irgen::createFunction(IRGenModule &IGM,
     IGM.Module.getFunctionList().push_back(fn);
   }
 
-  ApplyIRLinkage({linkInfo.getLinkage(), linkInfo.getVisibility(), linkInfo.getDLLStorage()})
-      .to(fn);
+  ApplyIRLinkage({linkInfo.getLinkage(),
+                  linkInfo.getVisibility(),
+                  linkInfo.getDLLStorage()})
+      .to(fn, linkInfo.isForDefinition());
 
   llvm::AttrBuilder initialAttrs;
   IGM.constructInitialFnAttributes(initialAttrs, FuncOptMode);
@@ -2082,7 +2084,7 @@ llvm::GlobalVariable *swift::irgen::createVariable(
   ApplyIRLinkage({linkInfo.getLinkage(),
                   linkInfo.getVisibility(),
                   linkInfo.getDLLStorage()})
-      .to(var);
+      .to(var, linkInfo.isForDefinition());
   var->setAlignment(llvm::MaybeAlign(alignment.getValue()));
 
   // Everything externally visible is considered used in Swift.
@@ -4742,7 +4744,7 @@ IRGenModule::getAddrOfWitnessTableLazyAccessFunction(
   LinkInfo link = LinkInfo::get(*this, entity, forDefinition);
   entry = createFunction(*this, link, signature);
   ApplyIRLinkage({link.getLinkage(), link.getVisibility(), link.getDLLStorage()})
-      .to(entry);
+      .to(entry, link.isForDefinition());
   return entry;
 }
 

lib/IRGen/GenKeyPath.cpp

@@ -1329,7 +1329,7 @@ void IRGenModule::emitSILProperty(SILProperty *prop) {
       ApplyIRLinkage({linkInfo.getLinkage(),
                       linkInfo.getVisibility(),
                       llvm::GlobalValue::DLLExportStorageClass})
-          .to(GA);
+          .to(GA, linkInfo.isForDefinition());
     }
     return;
   }

lib/IRGen/GenMeta.cpp

@@ -3635,7 +3635,7 @@ static void emitObjCClassSymbol(IRGenModule &IGM,
       ptrTy->getElementType(), ptrTy->getAddressSpace(), link.getLinkage(),
       link.getName(), metadata, &IGM.Module);
   ApplyIRLinkage({link.getLinkage(), link.getVisibility(), link.getDLLStorage()})
-      .to(alias);
+      .to(alias, link.isForDefinition());
 }
 
 /// Emit the type metadata or metadata template for a class.

test/IRGen/Inputs/comdat1.swift

@@ -0,0 +1,5 @@
+private final class C {}
+public func f() {
+  var cs: [C] = []
+  cs.append(C())
+}

test/IRGen/Inputs/comdat2.swift

@@ -0,0 +1,4 @@
+public func g() {
+  var a: [Int] = []
+  a.append(1)
+}

test/IRGen/comdat.swift

@@ -0,0 +1,14 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-ir %S/Inputs/comdat1.swift %S/Inputs/comdat2.swift -O -num-threads 1 -module-name comdat -o %t/comdat1.ll -o %t/comdat2.ll
+// RUN: %FileCheck -check-prefix CHECK-1 %s < %t/comdat1.ll
+// RUN: %FileCheck -check-prefix CHECK-2 %s < %t/comdat2.ll
+
+// REQUIRES: OS=windows-msvc
+
+// Ensure that the definition is marked as COMDAT
+// CHECK-1: "$s6comdat1C33_{{.*}}LLCMa" = comdat any
+// CHECK-1: "$s6comdat1C33_{{.*}}LLCMn" = comdat any
+
+// Ensure that no foward declaration is emitted
+// CHECK-2-NOT: "$s6comdat1C33_{{.*}}LLCMa" = comdat any
+// CHECK-2-NOT: "$s6comdat1C33_{{.*}}LLCMn" = comdat any