Skip to content

[compiler-rt] Switch LLD specific tests to a more precise option #69781

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
Oct 21, 2023

Conversation

mstorsjo
Copy link
Member

Prefer the new flag -lld-allow-duplicate-weak (which was added recently in a67ae8c), over the old -lldmingw.

The -lldmingw option enables a number of different behaviours, while this test only is interested in one aspect of them.

This should allow making -lldmingw more strict with not enabling MSVC specific behaviours like inferring lib directories automatically from the environment, as being pursued in https://reviews.llvm.org/D144084.

Prefer the new flag -lld-allow-duplicate-weak (which was added
recently in a67ae8c), over the old
-lldmingw.

The -lldmingw option enables a number of different behaviours,
while this test only is interested in one aspect of them.

This should allow making -lldmingw more strict with not enabling
MSVC specific behaviours like inferring lib directories automatically
from the environment, as being pursued in https://reviews.llvm.org/D144084.
@mstorsjo mstorsjo requested a review from MaskRay October 20, 2023 21:23
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Oct 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2023

@llvm/pr-subscribers-pgo

Author: Martin Storsjö (mstorsjo)

Changes

Prefer the new flag -lld-allow-duplicate-weak (which was added recently in a67ae8c), over the old -lldmingw.

The -lldmingw option enables a number of different behaviours, while this test only is interested in one aspect of them.

This should allow making -lldmingw more strict with not enabling MSVC specific behaviours like inferring lib directories automatically from the environment, as being pursued in https://reviews.llvm.org/D144084.


Full diff: https://github.com/llvm/llvm-project/pull/69781.diff

1 Files Affected:

  • (modified) compiler-rt/test/profile/Windows/coverage-weak-lld.cpp (+2-2)
diff --git a/compiler-rt/test/profile/Windows/coverage-weak-lld.cpp b/compiler-rt/test/profile/Windows/coverage-weak-lld.cpp
index 338be8e979c564e..e239d809640ff94 100644
--- a/compiler-rt/test/profile/Windows/coverage-weak-lld.cpp
+++ b/compiler-rt/test/profile/Windows/coverage-weak-lld.cpp
@@ -14,7 +14,7 @@
 // RUN: llvm-profdata show %t.profraw --all-functions | FileCheck %s --check-prefix=PROFILE1
 
 /// link.exe does not support weak overridding weak.
-// RUN: %clang_profgen -fcoverage-mapping -fuse-ld=lld -Wl,-lldmingw,-opt:ref %t0.o %t2.o -o %t2
+// RUN: %clang_profgen -fcoverage-mapping -fuse-ld=lld -Wl,-lld-allow-duplicate-weak,-opt:ref %t0.o %t2.o -o %t2
 // RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t2 | FileCheck %s --check-prefix=CHECK2
 // RUN: llvm-profdata show %t.profraw --all-functions | FileCheck %s --check-prefix=PROFILE2
 
@@ -30,7 +30,7 @@
 // RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t1 | FileCheck %s --check-prefix=CHECK1
 // RUN: llvm-profdata show %t.profraw --all-functions | FileCheck %s --check-prefix=PROFILE1
 
-// RUN: %clang_profgen -fcoverage-mapping -fuse-ld=lld -Wl,-lldmingw,-opt:ref %t0.o %t2.o -o %t2
+// RUN: %clang_profgen -fcoverage-mapping -fuse-ld=lld -Wl,-lld-allow-duplicate-weak,-opt:ref %t0.o %t2.o -o %t2
 // RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t2 | FileCheck %s --check-prefix=CHECK2
 // RUN: llvm-profdata show %t.profraw --all-functions | FileCheck %s --check-prefix=PROFILE2
 

@mstorsjo mstorsjo merged commit 8822eaa into llvm:main Oct 21, 2023
@mstorsjo mstorsjo deleted the profile-test-lld-weak branch October 21, 2023 06:42
mstorsjo added a commit that referenced this pull request Oct 21, 2023
In mingw mode, all linker paths are passed explicitly to the linker
by the compiler driver. Don't try to implicitly add linker paths
from the LIB environment variable or by detecting an MSVC
installation directory.

If the /winsysroot command line parameter is explicitly passed to
lld-link while /lldmingw is specified, it could be considered reasonable
to actually include those paths. However, modifying the code to
handle only the /winsysroot case but not the other ones, when the
mingw mode has been enabled, seems like much more code complexity
for a mostly hypothetical case.

Add a test for this when case when using LIB. (The code paths for
trying to detect an MSVC installation aren't really regression tested.)

Also fix an issue in the existing test for "Check that when /winsysroot
is specified, %LIB% is ignored.", where the LIB variable pointed
to a nonexistent directory, so the test would pass even if /winsysroot
wouldn't be specified.

Reland this after #68077 and
#69781 - the compiler-rt test
that used -lldmingw in MSVC environments has been updated to use a more
specific option.

Differential Revision: https://reviews.llvm.org/D144084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants