Merge pull request #75097 from hamishknight/windows-coverage

[Driver] Pass `-lld-allow-duplicate-weak` for coverage on Windows

Author: hamishknight

lib/Driver/WindowsToolChains.cpp

@@ -71,10 +71,36 @@ toolchains::Windows::constructInvocation(const DynamicLinkJobAction &job,
     llvm_unreachable("invalid link kind");
   }
 
+  // Check to see whether we need to use lld as the linker.
+  auto requiresLLD = [&]{
+    if (const Arg *A = context.Args.getLastArg(options::OPT_use_ld)) {
+      return llvm::StringSwitch<bool>(A->getValue())
+        .Cases("lld", "lld.exe", "lld-link", "lld-link.exe", true)
+        .Default(false);
+    }
+    // Force to use lld for LTO on Windows because we don't support link LTO or
+    // something else except for lld LTO at this time.
+    if (context.OI.LTOVariant != OutputInfo::LTOKind::None) {
+      return true;
+    }
+    // Profiling currently relies on the ability to emit duplicate weak
+    // symbols across translation units and having the linker coalesce them.
+    // Unfortunately link.exe does not support this, so require lld-link
+    // for now, which supports the behavior via a flag.
+    // TODO: Once we've changed coverage to no longer rely on emitting
+    // duplicate weak symbols (rdar://131295678), we can remove this.
+    if (context.Args.getLastArg(options::OPT_profile_generate)) {
+      return true;
+    }
+    return false;
+  }();
+
   // Select the linker to use.
   std::string Linker;
   if (const Arg *A = context.Args.getLastArg(options::OPT_use_ld)) {
     Linker = A->getValue();
+  } else if (requiresLLD) {
+    Linker = "lld";
   }
 
   switch (context.OI.LTOVariant) {
@@ -88,12 +114,6 @@ toolchains::Windows::constructInvocation(const DynamicLinkJobAction &job,
     break;
   }
 
-  if (Linker.empty() && context.OI.LTOVariant != OutputInfo::LTOKind::None) {
-    // Force to use lld for LTO on Windows because we don't support link LTO or
-    // something else except for lld LTO at this time.
-    Linker = "lld";
-  }
-
   if (!Linker.empty())
     Arguments.push_back(context.Args.MakeArgString("-fuse-ld=" + Linker));
 
@@ -176,6 +196,14 @@ toolchains::Windows::constructInvocation(const DynamicLinkJobAction &job,
     Arguments.push_back(context.Args.MakeArgString(
         Twine({"-include:", llvm::getInstrProfRuntimeHookVarName()})));
     Arguments.push_back(context.Args.MakeArgString("-lclang_rt.profile"));
+
+    // FIXME(rdar://131295678): Currently profiling requires the ability to
+    // emit duplicate weak symbols. Assuming we're using lld, pass
+    // -lld-allow-duplicate-weak to enable this behavior.
+    if (requiresLLD) {
+      Arguments.push_back("-Xlinker");
+      Arguments.push_back("-lld-allow-duplicate-weak");
+    }
   }
 
   context.Args.AddAllArgs(Arguments, options::OPT_Xlinker);

test/Profiler/coverage_inlined_counters.swift

@@ -0,0 +1,28 @@
+// RUN: %empty-directory(%t/out)
+// RUN: split-file %s %t
+
+// rdar://129337999 - Make sure we can link without issues.
+
+// The legacy driver does not support doing this in one invocation, so split it
+// up for compatibility with both the legacy and new driver.
+// RUN: %target-build-swift -c %t/a.swift -profile-generate -profile-coverage-mapping -parse-as-library -module-name A -o %t/out/a.swift.o
+// RUN: %target-build-swift -emit-module %t/a.swift -profile-generate -profile-coverage-mapping -parse-as-library -module-name A -emit-module-path %t/out/A.swiftmodule
+
+// RUN: %target-build-swift %t/b.swift -profile-generate -profile-coverage-mapping -o %t/main -module-name B -I %t/out %t/out/a.swift.o
+
+// Test again using only the old driver.
+// RUN: %empty-directory(%t/out)
+// RUN: env SWIFT_USE_OLD_DRIVER=1 %target-build-swift -c %t/a.swift -profile-generate -profile-coverage-mapping -parse-as-library -module-name A -o %t/out/a.swift.o
+// RUN: env SWIFT_USE_OLD_DRIVER=1 %target-build-swift -emit-module %t/a.swift -profile-generate -profile-coverage-mapping -parse-as-library -module-name A -emit-module-path %t/out/A.swiftmodule
+// RUN: env SWIFT_USE_OLD_DRIVER=1 %target-build-swift %t/b.swift -profile-generate -profile-coverage-mapping -o %t/main -module-name B -I %t/out %t/out/a.swift.o
+
+// REQUIRES: profile_runtime
+
+//--- a.swift
+@_transparent
+public func foo() {}
+
+//--- b.swift
+import A
+
+foo()

test/Profiler/coverage_inlined_counters_exec.swift

@@ -0,0 +1,33 @@
+// RUN: %empty-directory(%t/out)
+// RUN: split-file %s %t
+
+// rdar://129337999 - Make sure we can link without issues.
+
+// The legacy driver does not support doing this in one invocation, so split it
+// up for compatibility with both the legacy and new driver.
+// RUN: %target-build-swift -c %t/a.swift -profile-generate -profile-coverage-mapping -parse-as-library -module-name A -o %t/out/a.swift.o
+// RUN: %target-build-swift -emit-module %t/a.swift -profile-generate -profile-coverage-mapping -parse-as-library -module-name A -emit-module-path %t/out/A.swiftmodule
+
+// RUN: %target-build-swift %t/b.swift -profile-generate -profile-coverage-mapping -o %t/main -module-name B -I %t/out %t/out/a.swift.o
+
+// RUN: %target-codesign %t/main
+// RUN: env %env-LLVM_PROFILE_FILE=%t/default.profraw %target-run %t/main
+
+// RUN: %llvm-profdata merge %t/default.profraw -o %t/default.profdata
+
+// The implicit-check-not here ensures we match all recorded line counts.
+// RUN: %llvm-cov show %t/main -instr-profile=%t/default.profdata | %FileCheck --implicit-check-not "{{[ ]*\|[ ]*[0-9]+}}" %s
+
+// REQUIRES: profile_runtime
+// REQUIRES: executable_test
+
+//--- a.swift
+@_transparent
+public func foo() {}
+// CHECK: 1|public func foo() {}
+
+//--- b.swift
+import A
+
+foo()
+// CHECK: 1|foo()

test/Profiler/lit.local.cfg

@@ -0,0 +1,6 @@
+# Make a local copy of the environment.
+config.environment = dict(config.environment)
+
+# Prefer the new driver for profiling tests
+del config.environment['SWIFT_USE_OLD_DRIVER']
+del config.environment['SWIFT_AVOID_WARNING_USING_OLD_DRIVER']