Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions lib/Driver/WindowsToolChains.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));

Expand Down Expand Up @@ -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);
Expand Down
28 changes: 28 additions & 0 deletions test/Profiler/coverage_inlined_counters.swift
Original file line number Diff line number Diff line change
@@ -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()
33 changes: 33 additions & 0 deletions test/Profiler/coverage_inlined_counters_exec.swift
Original file line number Diff line number Diff line change
@@ -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()
6 changes: 6 additions & 0 deletions test/Profiler/lit.local.cfg
Original file line number Diff line number Diff line change
@@ -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']