Skip to content

[HotColdSplit] Don't outline musttail calls. #133177

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

Conversation

ahmedbougacha
Copy link
Member

musttail calls have several restrictions, generally enforcing matching calling conventions between the caller parent and musttail callee. We can't usually honor them, because the extracted function has a different signature altogether, taking inputs/outputs and returning a control-flow identifier rather than the actual return value.

Don't consider musttail calls as valid for extraction.

musttail calls have several restrictions, generally enforcing matching
calling conventions between the caller parent and musttail callee.
We can't usually honor them, because the extracted function has a
different signature altogether, taking inputs/outputs and returning
a control-flow identifier rather than the actual return value.

Don't consider musttail calls as valid for extraction.
@llvmbot
Copy link
Member

llvmbot commented Mar 26, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Ahmed Bougacha (ahmedbougacha)

Changes

musttail calls have several restrictions, generally enforcing matching calling conventions between the caller parent and musttail callee. We can't usually honor them, because the extracted function has a different signature altogether, taking inputs/outputs and returning a control-flow identifier rather than the actual return value.

Don't consider musttail calls as valid for extraction.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/CodeExtractor.cpp (+8)
  • (added) llvm/test/Transforms/HotColdSplit/musttail.ll (+53)
diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index 7277603b3ec2b..216db3a4570f7 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -170,6 +170,14 @@ static bool isBlockValidForExtraction(const BasicBlock &BB,
     }
 
     if (const CallInst *CI = dyn_cast<CallInst>(I)) {
+      // musttail calls have several restrictions, generally enforcing matching
+      // calling conventions between the caller parent and musttail callee.
+      // We can't usually honor them, because the extracted function has a
+      // different signature altogether, taking inputs/outputs and returning
+      // a control-flow identifier rather than the actual return value.
+      if (CI->isMustTailCall())
+        return false;
+
       if (const Function *F = CI->getCalledFunction()) {
         auto IID = F->getIntrinsicID();
         if (IID == Intrinsic::vastart) {
diff --git a/llvm/test/Transforms/HotColdSplit/musttail.ll b/llvm/test/Transforms/HotColdSplit/musttail.ll
new file mode 100644
index 0000000000000..b7108bd6e6cd9
--- /dev/null
+++ b/llvm/test/Transforms/HotColdSplit/musttail.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes=hotcoldsplit -hotcoldsplit-threshold=0 -S < %s | FileCheck %s
+
+; musttail calls can't be outlined, because they have several restrictions
+; enforcing matching calling conventions between the parent and callee.
+
+define void @test_musttail(i1 %b) {
+; CHECK-LABEL: define void @test_musttail(
+; CHECK-SAME: i1 [[B:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br i1 [[B]], label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
+; CHECK:       [[IF_THEN]]:
+; CHECK-NEXT:    ret void
+; CHECK:       [[IF_ELSE]]:
+; CHECK-NEXT:    musttail call void @target(i1 [[B]])
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %b, label %if.then, label %if.else
+
+if.then:
+  ret void
+
+if.else:
+  musttail call void @target(i1 %b)
+  ret void
+}
+
+define void @test_tail(i1 %b) {
+; CHECK-LABEL: define void @test_tail(
+; CHECK-SAME: i1 [[B:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br i1 [[B]], label %[[IF_THEN:.*]], label %[[CODEREPL:.*]]
+; CHECK:       [[IF_THEN]]:
+; CHECK-NEXT:    ret void
+; CHECK:       [[CODEREPL]]:
+; CHECK-NEXT:    call void @test_tail.cold.1(i1 [[B]]) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT:    br label %[[IF_ELSE_RET:.*]]
+; CHECK:       [[IF_ELSE_RET]]:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %b, label %if.then, label %if.else
+
+if.then:
+  ret void
+
+if.else:
+  tail call void @target(i1 %b)
+  ret void
+}
+
+declare void @target() cold

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@ahmedbougacha ahmedbougacha merged commit b394bab into llvm:main Mar 28, 2025
13 checks passed
@ahmedbougacha ahmedbougacha deleted the users/ahmedbougacha/hotcoldsplit-musttail branch March 28, 2025 23:18
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 28, 2025

LLVM Buildbot has detected a new failure on builder lldb-arm-ubuntu running on linaro-lldb-arm-ubuntu while building llvm at step 6 "test".

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

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-api :: macosx/corefile-exception-reason/TestCorefileExceptionReason.py (993 of 2932)
UNSUPPORTED: lldb-api :: macosx/ctf/TestCTF.py (994 of 2932)
UNSUPPORTED: lldb-api :: macosx/debugserver-exit-code/TestDebugServerExitCode.py (995 of 2932)
UNSUPPORTED: lldb-api :: macosx/delay-init-dependency/TestDelayInitDependency.py (996 of 2932)
PASS: lldb-api :: lldbutil-tests/failed-to-hit-breakpoint/TestLLDBUtilFailedToHitBreakpoint.py (997 of 2932)
UNSUPPORTED: lldb-api :: macosx/dyld-trie-symbols/TestDyldTrieSymbols.py (998 of 2932)
UNSUPPORTED: lldb-api :: macosx/early-process-launch/TestEarlyProcessLaunch.py (999 of 2932)
UNSUPPORTED: lldb-api :: macosx/expedited-thread-pcs/TestExpeditedThreadPCs.py (1000 of 2932)
UNSUPPORTED: lldb-api :: macosx/find-app-in-bundle/TestFindAppInBundle.py (1001 of 2932)
UNSUPPORTED: lldb-api :: macosx/find-dsym/bundle-with-dot-in-filename/TestBundleWithDotInFilename.py (1002 of 2932)
FAIL: lldb-api :: macosx/duplicate-archive-members/TestDuplicateMembers.py (1003 of 2932)
******************** TEST 'lldb-api :: macosx/duplicate-archive-members/TestDuplicateMembers.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --arch armv8l --build-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/macosx/duplicate-archive-members -p TestDuplicateMembers.py
--
Exit Code: -11

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision b394babddcec466a4cb8f31ea94b99c1b934ea71)
  clang revision b394babddcec466a4cb8f31ea94b99c1b934ea71
  llvm revision b394babddcec466a4cb8f31ea94b99c1b934ea71
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_dsym (TestDuplicateMembers.BSDArchivesTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_dwarf (TestDuplicateMembers.BSDArchivesTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang-arm) :: test_dwo (TestDuplicateMembers.BSDArchivesTestCase)
----------------------------------------------------------------------
Ran 3 tests in 0.993s

OK (skipped=1)

--

********************
UNSUPPORTED: lldb-api :: macosx/find-dsym/deep-bundle/TestDeepBundle.py (1004 of 2932)
UNSUPPORTED: lldb-api :: macosx/format/TestFunctionNameWithoutArgs.py (1005 of 2932)
UNSUPPORTED: lldb-api :: macosx/function-starts/TestFunctionStarts.py (1006 of 2932)
UNSUPPORTED: lldb-api :: macosx/ignore_exceptions/TestIgnoredExceptions.py (1007 of 2932)
UNSUPPORTED: lldb-api :: macosx/indirect_symbol/TestIndirectSymbols.py (1008 of 2932)
UNSUPPORTED: lldb-api :: macosx/lc-note/addrable-bits/TestAddrableBitsCorefile.py (1009 of 2932)
UNSUPPORTED: lldb-api :: macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py (1010 of 2932)
UNSUPPORTED: lldb-api :: macosx/lc-note/kern-ver-str/TestKernVerStrLCNOTE.py (1011 of 2932)
UNSUPPORTED: lldb-api :: macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py (1012 of 2932)
UNSUPPORTED: lldb-api :: macosx/macCatalyst/TestMacCatalyst.py (1013 of 2932)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants