Merge pull request #37359 from CodaFi/one-definition-to-rule-them-all

CodaFi · web-flow · commit cb319fc4b0fd · 2021-05-25T08:50:36.000-07:00
Undo Force Load + Incremental Ban on Darwin Platforms
diff --git a/include/swift/AST/DiagnosticsDriver.def b/include/swift/AST/DiagnosticsDriver.def
@@ -119,9 +119,6 @@ ERROR(error_input_changed_during_build,none,
 ERROR(error_conflicting_options, none,
       "conflicting options '%0' and '%1'",
       (StringRef, StringRef))
-ERROR(error_option_not_supported, none,
-      "'%0' is not supported with '%1'",
-      (StringRef, StringRef))
 ERROR(error_requirement_not_met, none,
       "'%0' requires '%1'",
       (StringRef, StringRef))
diff --git a/include/swift/IRGen/Linking.h b/include/swift/IRGen/Linking.h
@@ -1417,6 +1417,7 @@ struct IRLinkage {
   static const IRLinkage InternalWeakODR;
   static const IRLinkage Internal;
 
+  static const IRLinkage ExternalCommon;
   static const IRLinkage ExternalImport;
   static const IRLinkage ExternalWeakImport;
   static const IRLinkage ExternalExport;
diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
@@ -251,27 +251,6 @@ static void validateSearchPathArgs(DiagnosticEngine &diags,
   }
 }
 
-static void validateAutolinkingArgs(DiagnosticEngine &diags,
-                                    const ArgList &args,
-                                    const llvm::Triple &T) {
-  auto *forceLoadArg = args.getLastArg(options::OPT_autolink_force_load);
-  if (!forceLoadArg)
-    return;
-  auto *incrementalArg = args.getLastArg(options::OPT_incremental);
-  if (!incrementalArg)
-    return;
-
-  if (T.supportsCOMDAT())
-    return;
-
-  // Note: -incremental can itself be overridden by other arguments later
-  // on, but since -autolink-force-load is a rare and not-really-recommended
-  // option it's not worth modeling that complexity here (or moving the
-  // check somewhere else).
-  diags.diagnose(SourceLoc(), diag::error_option_not_supported,
-                 forceLoadArg->getSpelling(), incrementalArg->getSpelling());
-}
-
 /// Perform miscellaneous early validation of arguments.
 static void validateArgs(DiagnosticEngine &diags, const ArgList &args,
                          const llvm::Triple &T) {
@@ -282,7 +261,6 @@ static void validateArgs(DiagnosticEngine &diags, const ArgList &args,
   validateDebugInfoArgs(diags, args);
   validateCompilationConditionArgs(diags, args);
   validateSearchPathArgs(diags, args);
-  validateAutolinkingArgs(diags, args, T);
   validateVerifyIncrementalDependencyArgs(diags, args);
 }
 
diff --git a/lib/FrontendTool/TBD.cpp b/lib/FrontendTool/TBD.cpp
@@ -84,7 +84,8 @@ static bool validateSymbols(DiagnosticEngine &diags,
     if (auto GV = dyn_cast<llvm::GlobalValue>(value)) {
       // Is this a symbol that should be listed?
       auto externallyVisible =
-          GV->hasExternalLinkage() && !GV->hasHiddenVisibility();
+          (GV->hasExternalLinkage() || GV->hasCommonLinkage())
+        && !GV->hasHiddenVisibility();
       if (!GV->isDeclaration() && externallyVisible) {
         // Is it listed?
         if (!symbolSet.erase(name))
diff --git a/lib/IRGen/IRGenModule.cpp b/lib/IRGen/IRGenModule.cpp
@@ -1300,18 +1300,6 @@ static bool replaceModuleFlagsEntry(llvm::LLVMContext &Ctx,
   llvm_unreachable("Could not replace old linker options entry?");
 }
 
-/// Returns true if the object file generated by \p IGM will be the "first"
-/// object file in the module. This lets us determine where to put a symbol
-/// that must be unique.
-static bool isFirstObjectFileInModule(IRGenModule &IGM) {
-  if (IGM.getSILModule().isWholeModule())
-    return IGM.IRGen.getPrimaryIGM() == &IGM;
-
-  auto *file = cast<FileUnit>(IGM.getSILModule().getAssociatedContext());
-  auto *containingModule = file->getParentModule();
-  return containingModule->getFiles().front() == file;
-}
-
 static bool
 doesTargetAutolinkUsingAutolinkExtract(const SwiftTargetInfo &TargetInfo,
                                        const llvm::Triple &Triple) {
@@ -1486,6 +1474,47 @@ AutolinkKind AutolinkKind::create(const SwiftTargetInfo &TargetInfo,
   return AutolinkKind::LLVMLinkerOptions;
 }
 
+static llvm::GlobalObject *createForceImportThunk(IRGenModule &IGM) {
+  llvm::SmallString<64> buf;
+  encodeForceLoadSymbolName(buf, IGM.IRGen.Opts.ForceLoadSymbolName);
+  if (IGM.Triple.isOSBinFormatMachO()) {
+    // On Mach-O targets, emit a common force-load symbol that resolves to
+    // a global variable so it will be coalesced at static link time into a
+    // single external symbol.
+    //
+    // Looks like C's tentative definitions are good for something after all.
+    auto ForceImportThunk =
+        new llvm::GlobalVariable(IGM.Module,
+                                 IGM.Int1Ty,
+                                 /*isConstant=*/false,
+                                 llvm::GlobalValue::CommonLinkage,
+                                 llvm::Constant::getNullValue(IGM.Int1Ty),
+                                 buf.str());
+    ApplyIRLinkage(IRLinkage::ExternalCommon).to(ForceImportThunk);
+    IGM.addUsedGlobal(ForceImportThunk);
+    return ForceImportThunk;
+  } else {
+    // On all other targets, emit an external symbol that resolves to a
+    // function definition. On Windows, linkonce_odr is basically a no-op and
+    // the COMDAT we set by applying linkage gives us the desired coalescing
+    // behavior.
+    auto ForceImportThunk =
+        llvm::Function::Create(llvm::FunctionType::get(IGM.VoidTy, false),
+                               llvm::GlobalValue::LinkOnceODRLinkage, buf,
+                               &IGM.Module);
+    ForceImportThunk->setAttributes(IGM.constructInitialAttributes());
+    ApplyIRLinkage(IRLinkage::ExternalExport).to(ForceImportThunk);
+    if (IGM.Triple.supportsCOMDAT())
+      if (auto *GO = cast<llvm::GlobalObject>(ForceImportThunk))
+        GO->setComdat(IGM.Module.getOrInsertComdat(ForceImportThunk->getName()));
+
+    auto BB = llvm::BasicBlock::Create(IGM.getLLVMContext(), "", ForceImportThunk);
+    llvm::IRBuilder<> IRB(BB);
+    IRB.CreateRetVoid();
+    return ForceImportThunk;
+  }
+}
+
 void IRGenModule::emitAutolinkInfo() {
   auto Autolink =
       AutolinkKind::create(TargetInfo, Triple, IRGen.Opts.LLVMLTOKind);
@@ -1504,23 +1533,8 @@ void IRGenModule::emitAutolinkInfo() {
 
   Autolink.writeEntries(Entries, Metadata, *this);
 
-  if (!IRGen.Opts.ForceLoadSymbolName.empty() &&
-      (Triple.supportsCOMDAT() || isFirstObjectFileInModule(*this))) {
-    llvm::SmallString<64> buf;
-    encodeForceLoadSymbolName(buf, IRGen.Opts.ForceLoadSymbolName);
-    auto ForceImportThunk =
-        llvm::Function::Create(llvm::FunctionType::get(VoidTy, false),
-                               llvm::GlobalValue::ExternalLinkage, buf,
-                               &Module);
-    ForceImportThunk->setAttributes(constructInitialAttributes());
-    ApplyIRLinkage(IRLinkage::ExternalExport).to(ForceImportThunk);
-    if (Triple.supportsCOMDAT())
-      if (auto *GO = cast<llvm::GlobalObject>(ForceImportThunk))
-        GO->setComdat(Module.getOrInsertComdat(ForceImportThunk->getName()));
-
-    auto BB = llvm::BasicBlock::Create(getLLVMContext(), "", ForceImportThunk);
-    llvm::IRBuilder<> IRB(BB);
-    IRB.CreateRetVoid();
+  if (!IRGen.Opts.ForceLoadSymbolName.empty()) {
+    (void) createForceImportThunk(*this);
   }
 }
 
diff --git a/lib/IRGen/Linking.cpp b/lib/IRGen/Linking.cpp
@@ -50,6 +50,12 @@ const IRLinkage IRLinkage::Internal = {
   llvm::GlobalValue::DefaultStorageClass,
 };
 
+const IRLinkage IRLinkage::ExternalCommon = {
+  llvm::GlobalValue::CommonLinkage,
+  llvm::GlobalValue::DefaultVisibility,
+  llvm::GlobalValue::DLLExportStorageClass,
+};
+
 const IRLinkage IRLinkage::ExternalImport = {
   llvm::GlobalValue::ExternalLinkage,
   llvm::GlobalValue::DefaultVisibility,
diff --git a/lib/TBDGen/TBDGen.cpp b/lib/TBDGen/TBDGen.cpp
@@ -1186,9 +1186,6 @@ static bool hasLinkerDirective(Decl *D) {
 }
 
 void TBDGenVisitor::visitFile(FileUnit *file) {
-  if (file == SwiftModule->getFiles()[0])
-    addFirstFileSymbols();
-
   SmallVector<Decl *, 16> decls;
   file->getTopLevelDecls(decls);
 
@@ -1202,6 +1199,9 @@ void TBDGenVisitor::visitFile(FileUnit *file) {
 }
 
 void TBDGenVisitor::visit(const TBDGenDescriptor &desc) {
+  // Add any autolinking force_load symbols.
+  addFirstFileSymbols();
+  
   if (auto *singleFile = desc.getSingleFile()) {
     assert(SwiftModule == singleFile->getParentModule() &&
            "mismatched file and module");
@@ -1230,6 +1230,7 @@ void TBDGenVisitor::visit(const TBDGenDescriptor &desc) {
     // Diagnose module name that cannot be found
     ctx.Diags.diagnose(SourceLoc(), diag::unknown_swift_module_name, Name);
   }
+
   // Collect symbols in each module.
   llvm::for_each(Modules, [&](ModuleDecl *M) {
     for (auto *file : M->getFiles()) {
diff --git a/test/Driver/autolink-force-load-comdat.swift b/test/Driver/autolink-force-load-comdat.swift
diff --git a/test/Driver/autolink-force-load-no-comdat.swift b/test/Driver/autolink-force-load-no-comdat.swift
diff --git a/test/IRGen/autolink-force-link.swift b/test/IRGen/autolink-force-link.swift
@@ -3,27 +3,25 @@
 // RUN: %swift -disable-legacy-type-info -target x86_64-apple-macosx10.9 -parse-stdlib -autolink-force-load -module-name TEST -module-link-name TEST -emit-ir %s %S/../Inputs/empty.swift | %FileCheck -check-prefix=CHECK-WMO %s
 
 // CHECK-WMO: source_filename = "<swift-imported-modules>"
-// CHECK-WMO: define void @"_swift_FORCE_LOAD_$_TEST"()
+// CHECK-WMO: @"_swift_FORCE_LOAD_$_TEST"
 // CHECK-WMO-NOT: source_filename
 
 
 // RUN: %swift -disable-legacy-type-info -target x86_64-apple-macosx10.9 -parse-stdlib -autolink-force-load -module-name TEST -module-link-name TEST -emit-ir -num-threads 1 %s %S/../Inputs/empty.swift | %FileCheck -check-prefix=CHECK-WMO-THREADED %s
 
 // CHECK-WMO-THREADED: source_filename = "<swift-imported-modules>"
-// CHECK-WMO-THREADED: define void @"_swift_FORCE_LOAD_$_TEST"()
+// CHECK-WMO-THREADED: @"_swift_FORCE_LOAD_$_TEST"
 // CHECK-WMO-THREADED: source_filename = "<swift-imported-modules>"
-// CHECK-WMO-THREADED-NOT: _swift_FORCE_LOAD_$_TEST
-// CHECK-WMO-THREADED-NOT: source_filename
-
+// CHECK-WMO-THREADED: @"_swift_FORCE_LOAD_$_TEST"
 
 // RUN: %swift -disable-legacy-type-info -target x86_64-apple-macosx10.9 -parse-stdlib -autolink-force-load -module-name TEST -module-link-name TEST -emit-ir -primary-file %s %S/../Inputs/empty.swift | %FileCheck -check-prefix=CHECK-SINGLE-FILE-FIRST %s
 // RUN: %swift -disable-legacy-type-info -target x86_64-apple-macosx10.9 -parse-stdlib -autolink-force-load -module-name TEST -module-link-name TEST -emit-ir %S/../Inputs/empty.swift -primary-file %s | %FileCheck -check-prefix=CHECK-SINGLE-FILE-SECOND %s
 
 // CHECK-SINGLE-FILE-FIRST: source_filename = "<swift-imported-modules>"
-// CHECK-SINGLE-FILE-FIRST: define void @"_swift_FORCE_LOAD_$_TEST"()
+// CHECK-SINGLE-FILE-FIRST: @"_swift_FORCE_LOAD_$_TEST"
 // CHECK-SINGLE-FILE-FIRST-NOT: source_filename
 
 // CHECK-SINGLE-FILE-SECOND: source_filename = "<swift-imported-modules>"
-// CHECK-SINGLE-FILE-SECOND-NOT: _swift_FORCE_LOAD_$_TEST
+// CHECK-SINGLE-FILE-SECOND: @"_swift_FORCE_LOAD_$_TEST"
 // CHECK-SINGLE-FILE-SECOND-NOT: source_filename
 
diff --git a/test/Incremental/Inputs/autolinking/AutolinkingTest.framework/AutolinkingTest b/test/Incremental/Inputs/autolinking/AutolinkingTest.framework/AutolinkingTest
diff --git a/test/Incremental/Inputs/autolinking/AutolinkingTest.framework/Headers/AutolinkingTest.h b/test/Incremental/Inputs/autolinking/AutolinkingTest.framework/Headers/AutolinkingTest.h
@@ -0,0 +1,3 @@
+struct Test {
+  int value;
+};
diff --git a/test/Incremental/Inputs/autolinking/AutolinkingTest.framework/Modules/module.modulemap b/test/Incremental/Inputs/autolinking/AutolinkingTest.framework/Modules/module.modulemap
@@ -0,0 +1,5 @@
+framework module AutolinkingTest {
+  umbrella header "AutolinkingTest.h"
+  export *
+  module * { export * }
+}
diff --git a/test/Incremental/Inputs/autolinking/autolinking-other.swift b/test/Incremental/Inputs/autolinking/autolinking-other.swift
@@ -0,0 +1 @@
+
diff --git a/test/Incremental/Inputs/autolinking/ofm.json b/test/Incremental/Inputs/autolinking/ofm.json
@@ -0,0 +1,18 @@
+{
+  "autolinking.swift": {
+    "object": "./autolinking.o",
+    "swift-dependencies": "./autolinking.swiftdeps"
+  },
+  "autolinking-overlay.swift": {
+    "object": "./autolinking-overlay.o",
+    "swift-dependencies": "./autolinking-overlay.swiftdeps"
+  },
+  "autolinking-other.swift": {
+    "object": "./autolinking-other.o",
+    "swift-dependencies": "./autolinking-other.swiftdeps"
+  },
+  "": {
+    "swift-dependencies": "./autolinking~buildrecord.swiftdeps"
+  }
+}
+
diff --git a/test/Incremental/autolinking-overlay.swift b/test/Incremental/autolinking-overlay.swift
@@ -0,0 +1,19 @@
+// This test shares inputs with its cousin autolinking.swift
+// This one specifically tests an overlay-esque build.
+
+// RUN: %empty-directory(%t)
+// RUN: cp %s %t
+// RUN: cp -r %S/Inputs/autolinking/* %t
+// RUN: cd %t
+
+// RUN: %target-build-swift-dylib(%t/%target-library-name(AutolinkingTest)) -autolink-force-load -module-link-name swiftAutolinkingTest -incremental -driver-show-incremental -module-name AutolinkingTest -output-file-map ofm.json -F %t -import-underlying-module autolinking-overlay.swift autolinking-other.swift
+
+// Make sure `swift_FORCE_LOAD_$_swiftAutolinkingTest` appears in all objects
+// RUN: llvm-readobj -symbols -coff-exports %t/autolinking-overlay.o | %FileCheck %s
+// RUN: llvm-readobj -symbols -coff-exports %t/autolinking-other.o | %FileCheck %s
+// RUN: llvm-readobj -symbols -coff-exports %t/%target-library-name(AutolinkingTest) | %FileCheck %s
+
+// Emulate an overlay build by importing content from the underlying module.
+extension Test { }
+
+// CHECK: _swift_FORCE_LOAD_$_swiftAutolinkingTest
diff --git a/test/Incremental/autolinking.swift b/test/Incremental/autolinking.swift
@@ -0,0 +1,16 @@
+// This test shares inputs with its cousin autolinking-overlay.swift
+// This one specifically tests the bare minimum: two empty swift files.
+
+// RUN: %empty-directory(%t)
+// RUN: cp %s %t
+// RUN: cp -r %S/Inputs/autolinking/* %t
+// RUN: cd %t
+
+// RUN: %target-build-swift-dylib(%t/%target-library-name(AutolinkingTest)) -autolink-force-load -module-link-name swiftAutolinkingTest -incremental -driver-show-incremental -module-name AutolinkingTest -output-file-map ofm.json autolinking.swift autolinking-other.swift
+
+// Make sure `swift_FORCE_LOAD_$_swiftAutolinkingTest` appears in all objects
+// RUN: llvm-readobj -symbols -coff-exports %t/autolinking.o | %FileCheck %s
+// RUN: llvm-readobj -symbols -coff-exports %t/autolinking-other.o | %FileCheck %s
+// RUN: llvm-readobj -symbols -coff-exports %t/%target-library-name(AutolinkingTest) | %FileCheck %s
+
+// CHECK: _swift_FORCE_LOAD_$_swiftAutolinkingTest
diff --git a/test/Serialization/autolinking.swift b/test/Serialization/autolinking.swift
@@ -40,12 +40,8 @@ import someModule
 // NO-FORCE-LOAD-NOT: FORCE_LOAD
 // NO-FORCE-LOAD-NOT -lmodule
 // NO-FORCE-LOAD-NOT -lmagic
-// FORCE-LOAD: define{{( dllexport)?}} void @"_swift_FORCE_LOAD_$_module"() {{(#[0-9]+)?( comdat)?}} {
-// FORCE-LOAD:   ret void
-// FORCE-LOAD: }
-// FORCE-LOAD-HEX: define{{( dllexport)?}} void @"_swift_FORCE_LOAD_$306d6f64756c65"() {{(#[0-9]+)?( comdat)?}} {
-// FORCE-LOAD-HEX:   ret void
-// FORCE-LOAD-HEX: }
+// FORCE-LOAD: @"_swift_FORCE_LOAD_$_module"
+// FORCE-LOAD-HEX: @"_swift_FORCE_LOAD_$306d6f64756c65"
 
 // NO-FORCE-LOAD-CLIENT-NOT: FORCE_LOAD
 // FORCE-LOAD-CLIENT: @"_swift_FORCE_LOAD_$_module_$_autolinking" = weak_odr hidden constant void ()* @"_swift_FORCE_LOAD_$_module"
