Skip to content
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

[Driver][MSVC] Pass profile file to lld-link via -lto-sample-profile option #127442

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

tianleliu
Copy link
Contributor

@tianleliu tianleliu commented Feb 17, 2025

In SPGO lto mode, linker needs -lto-sample-profile option to set sample profile file.
Linux adds this option by transferring fprofile-sample-use to -plugin-opt=sample-profile=, which is alias of lto-sample-profile. (in clang\lib\Driver\ToolChains\CommonArgs.cpp: tools::addLTOOptions()).
But clang on Windows misses the transferring. So add it now.

SPGO in lto mode, linker needs -lto-sample-profile option to set
sample profile file. Linux adds this option by transfering
fprofile-sample-use but lld-link on Windows misses the transfering.
So add it now.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Feb 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 17, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: None (tianleliu)

Changes

SPGO in lto mode, linker needs -lto-sample-profile option to set sample profile file. Linux adds this option by transfering fprofile-sample-use but lld-link on Windows misses the transfering. So add it now.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/MSVC.cpp (+3)
  • (modified) clang/test/Driver/cl-link.c (+3)
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index bae41fc06c036..1c6854e3ef775 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -232,6 +232,9 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_fprofile_sample_use_EQ))
+    CmdArgs.push_back(Args.MakeArgString(std::string("-lto-sample-profile:") +
+                                         A->getValue()));
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
   // Control Flow Guard checks
diff --git a/clang/test/Driver/cl-link.c b/clang/test/Driver/cl-link.c
index 9bf8a8137926d..f1097ad0be9bf 100644
--- a/clang/test/Driver/cl-link.c
+++ b/clang/test/Driver/cl-link.c
@@ -71,3 +71,6 @@
 // RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -fuse-ld=lld -### -fsanitize=address 2>&1 | FileCheck --check-prefix=INFER-LLD %s
 // INFER-LLD: lld-link
 // INFER-LLD-NOT: INFERASANLIBS
+
+// RUN: %clang_cl --target=x86_64-unknown-windows-msvc -fuse-ld=lld -### -S -fprofile-sample-use=%S/Inputs/file.prof %s 2>&1 | FileCheck -check-prefix=CHECK-SAMPLE-PROFILE %s
+// CHECK-SAMPLE-PROFILE: "-lto-sample-profile:{{.*}}/file.prof"

@tianleliu tianleliu changed the title [SPGO][Driver][Win] Add lto-sample-profile option for lld-link [Driver][MSVC] Add lto-sample-profile option for lld-link Feb 17, 2025
@tianleliu tianleliu requested a review from HaohaiWen February 17, 2025 07:41
@@ -71,3 +71,6 @@
// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -fuse-ld=lld -### -fsanitize=address 2>&1 | FileCheck --check-prefix=INFER-LLD %s
// INFER-LLD: lld-link
// INFER-LLD-NOT: INFERASANLIBS

// RUN: %clang_cl --target=x86_64-unknown-windows-msvc -flto -fuse-ld=lld -### -S -fprofile-sample-use=%S/Inputs/file.prof %s 2>&1 | FileCheck -check-prefix=CHECK-SAMPLE-PROFILE %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use /Tc%s?
-S should be removed.
It's option for linker. I suggest first compile it to .obj then check it when using clang_cl to invoke linker.

@@ -232,6 +232,11 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
}
}

if (C.getDriver().isUsingLTO()) {
if (Arg *A = Args.getLastArg(options::OPT_fprofile_sample_use_EQ))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use getLastProfileSampleUseArg

@HaohaiWen
Copy link
Contributor

This tittle looks like you are adding a new option -lto-sample-profile for LLD.
It's recommended to rename the tittle so that other developer can easily know you are trying to pass profile file to lto backend via -lto-sample-profile.

@tianleliu tianleliu changed the title [Driver][MSVC] Add lto-sample-profile option for lld-link [Driver][MSVC] Pass profile file to lld-link via -lto-sample-profile option Feb 19, 2025
@tianleliu tianleliu requested a review from MaskRay February 19, 2025 03:28
@tianleliu tianleliu merged commit 6c39ee7 into llvm:main Feb 19, 2025
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 19, 2025

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu running on as-builder-9 while building clang at step 16 "test-check-lldb-api".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/5083

Here is the relevant piece of the build log for the reference
Step 16 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure)
...
PASS: lldb-api :: types/TestCharTypeExpr.py (1222 of 1231)
PASS: lldb-api :: types/TestIntegerType.py (1223 of 1231)
PASS: lldb-api :: types/TestRecursiveTypes.py (1224 of 1231)
PASS: lldb-api :: types/TestIntegerTypeExpr.py (1225 of 1231)
PASS: lldb-api :: python_api/watchpoint/watchlocation/TestTargetWatchAddress.py (1226 of 1231)
PASS: lldb-api :: types/TestShortType.py (1227 of 1231)
PASS: lldb-api :: types/TestLongTypes.py (1228 of 1231)
PASS: lldb-api :: types/TestShortTypeExpr.py (1229 of 1231)
PASS: lldb-api :: types/TestLongTypesExpr.py (1230 of 1231)
TIMEOUT: lldb-api :: python_api/process/cancel_attach/TestCancelAttach.py (1231 of 1231)
******************** TEST 'lldb-api :: python_api/process/cancel_attach/TestCancelAttach.py' FAILED ********************
Script:
--
/usr/bin/python3.12 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --libcxx-include-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/c++/v1 --libcxx-include-target-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/aarch64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib/aarch64-unknown-linux-gnu --arch aarch64 --build-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/lldb --compiler /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang --dsymutil /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --lldb-obj-root /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb --lldb-libs-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --platform-url connect://jetson-agx-2198.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot /mnt/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux --skip-category=lldb-server /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/python_api/process/cancel_attach -p TestCancelAttach.py
--
Exit Code: -9
Timeout: Reached timeout of 600 seconds

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 6c39ee717f03a0fe28f563d525fa5aff09804ba8)
  clang revision 6c39ee717f03a0fe28f563d525fa5aff09804ba8
  llvm revision 6c39ee717f03a0fe28f563d525fa5aff09804ba8

--
Command Output (stderr):
--
WARNING:root:Custom libc++ is not supported for remote runs: ignoring --libcxx arguments
FAIL: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_scripted_implementation (TestCancelAttach.AttachCancelTestCase.test_scripted_implementation)

--

********************
Slowest Tests:
--------------------------------------------------------------------------
600.04s: lldb-api :: python_api/process/cancel_attach/TestCancelAttach.py
180.95s: lldb-api :: commands/command/script_alias/TestCommandScriptAlias.py
70.56s: lldb-api :: commands/process/attach/TestProcessAttach.py
40.44s: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
34.86s: lldb-api :: functionalities/completion/TestCompletion.py
34.04s: lldb-api :: functionalities/single-thread-step/TestSingleThreadStepTimeout.py
26.58s: lldb-api :: python_api/watchpoint/watchlocation/TestTargetWatchAddress.py
20.82s: lldb-api :: commands/statistics/basic/TestStats.py
20.73s: lldb-api :: functionalities/gdb_remote_client/TestPlatformClient.py
19.31s: lldb-api :: functionalities/thread/state/TestThreadStates.py
18.17s: lldb-api :: commands/dwim-print/TestDWIMPrint.py
14.76s: lldb-api :: commands/expression/expr-in-syscall/TestExpressionInSyscall.py
14.69s: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
14.39s: lldb-api :: functionalities/inline-stepping/TestInlineStepping.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants