[MIPS][ISel] Fix musttail #161860

djtodoro · 2025-10-03T15:13:25Z

Properly handle clang::musttail attribute on MIPS backend.

It fixes: #161193

Properly handle clang::musttail attribute on MIPS backend.

llvmbot · 2025-10-03T15:14:03Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-mips

Author: Djordje Todorovic (djtodoro)

Changes

Properly handle clang::musttail attribute on MIPS backend.

It fixes: #161193

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

2 Files Affected:

(modified) llvm/lib/Target/Mips/MipsISelLowering.cpp (+4-3)
(added) llvm/test/CodeGen/Mips/musttail.ll (+38)

diff --git a/llvm/lib/Target/Mips/MipsISelLowering.cpp b/llvm/lib/Target/Mips/MipsISelLowering.cpp
index 881ba8e2f9eff..d79db127f01a2 100644
--- a/llvm/lib/Target/Mips/MipsISelLowering.cpp
+++ b/llvm/lib/Target/Mips/MipsISelLowering.cpp
@@ -3407,7 +3407,8 @@ MipsTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   // Check if it's really possible to do a tail call. Restrict it to functions
   // that are part of this compilation unit.
   bool InternalLinkage = false;
-  if (IsTailCall) {
+  bool IsMustTail = CLI.CB && CLI.CB->isMustTailCall();
+  if (IsTailCall && !IsMustTail) {
     IsTailCall = isEligibleForTailCallOptimization(
         CCInfo, StackSize, *MF.getInfo<MipsFunctionInfo>());
     if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee)) {
@@ -3416,9 +3417,9 @@ MipsTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
                      G->getGlobal()->hasPrivateLinkage() ||
                      G->getGlobal()->hasHiddenVisibility() ||
                      G->getGlobal()->hasProtectedVisibility());
-     }
+    }
   }
-  if (!IsTailCall && CLI.CB && CLI.CB->isMustTailCall())
+  if (!IsTailCall && IsMustTail)
     report_fatal_error("failed to perform tail call elimination on a call "
                        "site marked musttail");
 
diff --git a/llvm/test/CodeGen/Mips/musttail.ll b/llvm/test/CodeGen/Mips/musttail.ll
new file mode 100644
index 0000000000000..d5f457f6eb665
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/musttail.ll
@@ -0,0 +1,38 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=mips-unknown-linux-gnu < %s | FileCheck %s --check-prefix=MIPS32
+; RUN: llc -mtriple=mips64-unknown-linux-gnu < %s | FileCheck %s --check-prefix=MIPS64
+
+; Test musttail support for MIPS
+
+declare void @external_func()
+
+; Test basic musttail with external function
+define void @test_musttail_external() {
+; MIPS32-LABEL: test_musttail_external:
+; MIPS32:       # %bb.0:
+; MIPS32-NEXT:    j external_func
+; MIPS32-NEXT:    nop
+;
+; MIPS64-LABEL: test_musttail_external:
+; MIPS64:       # %bb.0:
+; MIPS64-NEXT:    j external_func
+; MIPS64-NEXT:    nop
+  musttail call void @external_func()
+  ret void
+}
+
+declare i32 @callee_args(i32 %a, i32 %b, i32 %c)
+
+define i32 @test_musttail_args(i32 %x, i32 %y, i32 %z) {
+; MIPS32-LABEL: test_musttail_args:
+; MIPS32:       # %bb.0:
+; MIPS32-NEXT:    j callee_args
+; MIPS32-NEXT:    nop
+;
+; MIPS64-LABEL: test_musttail_args:
+; MIPS64:       # %bb.0:
+; MIPS64-NEXT:    j callee_args
+; MIPS64-NEXT:    nop
+  %ret = musttail call i32 @callee_args(i32 %x, i32 %y, i32 %z)
+  ret i32 %ret
+}

kraj

LGTM

jrtc27 · 2025-10-07T04:49:28Z

llvm/lib/Target/Mips/MipsISelLowering.cpp

Uh don't we still need to check it's eligible?

Well, that thing confuses me in the case of musttail. On ARM, we do check it. Lets do it for MIPS as well.

@jrtc27 yep, it does make sense to still check it. I incorporated in the new commit (basically - do not check for the optional mips flag for enabling tail call optimizations in case of musttial, and I also enabled tail call opts for dso_local functions (1dcb9110612ab) ). Thanks for the review!

jrtc27

.

djtodoro · 2025-10-07T08:10:26Z

I checked the issues listed above, all cases from those still pass with the new patch.

github-actions · 2025-10-07T08:10:42Z

✅ With the latest revision this PR passed the C/C++ code formatter.

kraj · 2025-10-08T02:34:55Z

Please squash changes into initial commit

djtodoro · 2025-10-08T06:04:04Z

Please squash changes into initial commit

Per "Contributing to LLVM" Policy, we do not do force-push (and squash), we rather use incremental changes to address comments during review process. And, once the pull request is approved, we select the “Squash and merge” button.

kraj

my checks pass with this series as well.

kraj · 2025-10-08T06:50:51Z

Please squash changes into initial commit

Per "Contributing to LLVM" Policy, we do not do force-push (and squash), we rather use incremental changes to address comments during review process. And, once the pull request is approved, we select the “Squash and merge” button.

right, squashing is what I meant.

djtodoro · 2025-10-10T08:42:12Z

I will wait for @jrtc27 to have another look. :)

brad0 · 2025-10-10T09:08:11Z

@jrtc27 @yingopq Ping.

efriedma-quic · 2025-10-16T21:25:25Z

llvm/lib/Target/Mips/MipsISelLowering.h

I'd prefer to avoid using a default argument here, if there aren't too many callers.

efriedma-quic · 2025-10-16T21:30:12Z

llvm/lib/Target/Mips/MipsISelLowering.cpp

hasLocalLinkage overlaps with some of the other checks here.

Where does this list of checks come from? What are we trying to avoid here? I don't see any obvious reason why you can't tail-call an external function. If there is an issue, there should be a testcase with an explanation.

Hi @efriedma-quic.

hasLocalLinkage overlaps with some of the other checks here.

I agree, will address it.

Where does this list of checks come from? What are we trying to avoid here? I don't see any obvious reason why you can't tail-call an external function. If there is an issue, there should be a testcase with an explanation.

I was not sure as well, but after some investigation: The problem caused it was around $gp register. In PIC mode, calling external functions via tail call can cause issues with $gp register handling: https://reviews.llvm.org/D24763

Is the problem here similar to #98859? i.e. tail-call optimization conflicts with optimizations for setting up $gp?

Yep, seems very similar

efriedma-quic

Also, I'd like to see a little more test coverage. Particularly for calling a non-local function, and for more complicated arguments lists that involve memory.

efriedma-quic · 2025-10-17T17:31:24Z

llvm/lib/Target/Mips/MipsISelLowering.cpp

Can you clean up this InternalLinkage variable? The remaining usage doesn't seem like it's connected to this code.

Is musttail of a function pointer okay?

Can you clean up this InternalLinkage variable? The remaining usage doesn't seem like it's connected to this code.

yep, it makes sense

Is musttail of a function pointer okay?

yes, I will add a test case for it

efriedma-quic · 2025-10-20T20:22:47Z

llvm/lib/Target/Mips/MipsISelLowering.cpp

If there's a ABI problem, we need to reject musttail calls which would trigger that problem. A miscompile is a lot worse than a backend error.

If there's not an ABI problem, I'm not sure why we'd need these checks.

Well. I think that what we did in case of PPC is more appropriate fix for this.

ameisen · 2025-10-30T18:02:43Z

This PR doesn't fix (if it was intended to) sibling call optimizations.

It does change the code generation for them, however - but it maintains all of the stack adjustments as well as the fp/sp moves.

__attribute__((noinline))
int foo0()
{
	volatile int a;
	return a += 1;
}

int bar0()
{
	return foo0();
}

with tail calls disabled (-std=gnu++23 -O3 -march=mips32r6 -fno-PIC -mllvm -mips-tail-calls=0):

	addiu	$sp, $sp, -24
	sw	$ra, 20($sp)                    # 4-byte Folded Spill
	sw	$fp, 16($sp)                    # 4-byte Folded Spill
	move	$fp, $sp
	jal	_Z4foo0v
	nop
	move	$sp, $fp
	lw	$fp, 16($sp)                    # 4-byte Folded Reload
	lw	$ra, 20($sp)                    # 4-byte Folded Reload
	addiu	$sp, $sp, 24
	jrc	$ra

with tail calls enabled (-std=gnu++23 -O3 -march=mips32r6 -fno-PIC -mllvm -mips-tail-calls=1):

	addiu	$sp, $sp, -8
	sw	$ra, 4($sp)                     # 4-byte Folded Spill
	sw	$fp, 0($sp)                     # 4-byte Folded Spill
	move	$fp, $sp
	move	$sp, $fp
	lw	$fp, 0($sp)                     # 4-byte Folded Reload
	lw	$ra, 4($sp)                     # 4-byte Folded Reload
	j	_Z4foo0v
	addiu	$sp, $sp, 8

What is expected:

	j	_Z4foo0v
	nop

or

	bc	_Z4foo0v

So, it's moving the j to the end, but it's not cleaning up the stack changes that were around the jump. Everything in here but the j could be removed (with a nop, but the j should be replaced with a bc anyways), but this effectively is generating a rather messed up function.

A significant amount of logic for handling this issue is present in LowerCall for PPC and for AArch64 (and the rest), but is missing in the MIPS version.

djtodoro · 2025-10-31T08:55:52Z

This PR doesn't fix (if it was intended to) sibling call optimizations.

It does change the code generation for them, however - but it maintains all of the stack adjustments as well as the fp/sp moves.
__attribute__((noinline))
int foo0()
{
	volatile int a;
	return a += 1;
}

int bar0()
{
	return foo0();
}
with tail calls disabled (-std=gnu++23 -O3 -march=mips32r6 -fno-PIC -mllvm -mips-tail-calls=0):
	addiu	$sp, $sp, -24
	sw	$ra, 20($sp)                    # 4-byte Folded Spill
	sw	$fp, 16($sp)                    # 4-byte Folded Spill
	move	$fp, $sp
	jal	_Z4foo0v
	nop
	move	$sp, $fp
	lw	$fp, 16($sp)                    # 4-byte Folded Reload
	lw	$ra, 20($sp)                    # 4-byte Folded Reload
	addiu	$sp, $sp, 24
	jrc	$ra
with tail calls enabled (-std=gnu++23 -O3 -march=mips32r6 -fno-PIC -mllvm -mips-tail-calls=1):
	addiu	$sp, $sp, -8
	sw	$ra, 4($sp)                     # 4-byte Folded Spill
	sw	$fp, 0($sp)                     # 4-byte Folded Spill
	move	$fp, $sp
	move	$sp, $fp
	lw	$fp, 0($sp)                     # 4-byte Folded Reload
	lw	$ra, 4($sp)                     # 4-byte Folded Reload
	j	_Z4foo0v
	addiu	$sp, $sp, 8
What is expected:
	j	_Z4foo0v
	nop
or
	bc	_Z4foo0v
So, it's moving the j to the end, but it's not cleaning up the stack changes that were around the jump. Everything in here but the j could be removed (with a nop, but the j should be replaced with a bc anyways), but this effectively is generating a rather messed up function.

A significant amount of logic for handling this issue is present in LowerCall for PPC and for AArch64 (and the rest), but is missing in the MIPS version.

Hey @ameisen, thanks a lot. Yes, I agree, it is missing for MIPS case in the ISel Lowering, but I think this should be a separate github issue, and separate PR should be created for this. Can you please create an issue with the content from this comment? Thanks in advance.

ameisen · 2025-10-31T18:43:07Z

This PR doesn't fix (if it was intended to) sibling call optimizations.
It does change the code generation for them, however - but it maintains all of the stack adjustments as well as the fp/sp moves.
__attribute__((noinline))
int foo0()
{
	volatile int a;
	return a += 1;
}

int bar0()
{
	return foo0();
}
with tail calls disabled (-std=gnu++23 -O3 -march=mips32r6 -fno-PIC -mllvm -mips-tail-calls=0):
	addiu	$sp, $sp, -24
	sw	$ra, 20($sp)                    # 4-byte Folded Spill
	sw	$fp, 16($sp)                    # 4-byte Folded Spill
	move	$fp, $sp
	jal	_Z4foo0v
	nop
	move	$sp, $fp
	lw	$fp, 16($sp)                    # 4-byte Folded Reload
	lw	$ra, 20($sp)                    # 4-byte Folded Reload
	addiu	$sp, $sp, 24
	jrc	$ra
with tail calls enabled (-std=gnu++23 -O3 -march=mips32r6 -fno-PIC -mllvm -mips-tail-calls=1):
	addiu	$sp, $sp, -8
	sw	$ra, 4($sp)                     # 4-byte Folded Spill
	sw	$fp, 0($sp)                     # 4-byte Folded Spill
	move	$fp, $sp
	move	$sp, $fp
	lw	$fp, 0($sp)                     # 4-byte Folded Reload
	lw	$ra, 4($sp)                     # 4-byte Folded Reload
	j	_Z4foo0v
	addiu	$sp, $sp, 8
What is expected:
	j	_Z4foo0v
	nop
or
	bc	_Z4foo0v
So, it's moving the j to the end, but it's not cleaning up the stack changes that were around the jump. Everything in here but the j could be removed (with a nop, but the j should be replaced with a bc anyways), but this effectively is generating a rather messed up function.
A significant amount of logic for handling this issue is present in LowerCall for PPC and for AArch64 (and the rest), but is missing in the MIPS version.
Hey @ameisen, thanks a lot. Yes, I agree, it is missing for MIPS case in the ISel Lowering, but I think this should be a separate github issue, and separate PR should be created for this. Can you please create an issue with the content from this comment? Thanks in advance.

I'm trying to figure out how to word the issue, as without your PR clang doesn't even turn it into a tail call at all. Since sibling optimizations are based upon tail call optimizations (they're a subset of them) it complicates an issue as I'm not sure how to write an issue dependent upon a PR.

I created this issue for now: #165906.

ameisen · 2025-10-31T18:52:58Z

As a separate problem, with that simple code sample, if I add the musttail attribute, with your PR I am getting this error:

'musttail' attribute for this call is impossible because calls outside the current linkage unit cannot be tail called on MIPS

If I make foo0 static it works, but that shouldn't be necessary in this situation. I'm also confused as to why it's partially performing the tail call optimization without musttail, but musttail fails.

When it's static, though, it's just inlining it instead, ignoring the noinline attribute.

--

Edit: The conditions that you use to check for the validity of musttail differ from from the conditions used in LowerCall to determine is a tail-call is possible. You are stricter in what the linkage of the function must be.

efriedma-quic

LGTM

ameisen

Review related to the error that I encountered.

ameisen · 2025-11-11T00:33:12Z

clang/lib/CodeGen/CGCall.cpp

+          if (!FD->isDefined()) {
+            CGM.addUndefinedGlobalForTailCall({FD, Loc});
+          } else {
+            llvm::GlobalValue::LinkageTypes Linkage =


The conditions used here differ from the conditions used in MipsISelLowering. This causes it to be inconsistent and results in the error I provided in conversations.

Locally, I used this which still isn't correct but seems to work more consistently:

bool HasLocalLinkage = llvm::GlobalValue::isLocalLinkage(Linkage) || llvm::GlobalValue::isPrivateLinkage(Linkage); if (!HasLocalLinkage || llvm::GlobalValue::isWeakForLinker(Linkage))

ameisen · 2025-11-11T00:33:41Z

clang/lib/CodeGen/CodeGenModule.cpp

+          getDiags().Report(I.second, diag::err_mips_impossible_musttail) << 1;
+          continue;
+        }
+        llvm::GlobalValue::LinkageTypes Linkage =


As in CGCall.cpp, the conditions used here differ from the conditions used in MipsISelLowering.

brad0 · 2025-11-18T22:01:57Z

@djtodoro

[MIPS][ISel] Fix musttail

ac9497e

Properly handle clang::musttail attribute on MIPS backend.

djtodoro requested review from topperc and ukalappa-mips October 3, 2025 15:13

llvmbot added the backend:MIPS label Oct 3, 2025

brad0 requested a review from yingopq October 4, 2025 02:07

kraj approved these changes Oct 7, 2025

View reviewed changes

jrtc27 reviewed Oct 7, 2025

View reviewed changes

jrtc27 requested changes Oct 7, 2025

View reviewed changes

Address comments

9a77adf

Apply clang-format

1e26120

brad0 mentioned this pull request Oct 8, 2025

Enable tail call optimization on MIPS32 ziglang/zig#21332

Open

brad0 requested review from jrtc27 and kraj October 8, 2025 01:57

kraj approved these changes Oct 8, 2025

View reviewed changes

brad0 requested a review from efriedma-quic October 16, 2025 20:45

efriedma-quic reviewed Oct 16, 2025

View reviewed changes

address comments

20d634f

brad0 requested a review from efriedma-quic October 17, 2025 16:43

efriedma-quic reviewed Oct 17, 2025

View reviewed changes

Clean up and improve tests

eaa5678

brad0 requested a review from efriedma-quic October 19, 2025 04:40

djtodoro added 2 commits October 19, 2025 08:50

apply clang-format

54412fe

apply clang-format on Mips16ISelLowering.h

b9e145d

efriedma-quic reviewed Oct 20, 2025

View reviewed changes

Align with PPC solution

e4a3991

djtodoro requested a review from efriedma-quic October 29, 2025 17:51

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Oct 29, 2025

Apply clang-format and simplify code for err reporting

a4b960c

ameisen mentioned this pull request Oct 31, 2025

[LLVM][MIPS] Sibling calls push/pop stack #165906

Open

folkertdev mentioned this pull request Nov 9, 2025

riscv64: rustc-LLVM ERROR: failed to perform tail call elimination on a call site marked musttail rust-lang/rust#148748

Open

efriedma-quic approved these changes Nov 10, 2025

View reviewed changes

ameisen reviewed Nov 11, 2025

View reviewed changes

[MIPS][ISel] Fix musttail #161860

Are you sure you want to change the base?

[MIPS][ISel] Fix musttail #161860

Uh oh!

Conversation

djtodoro commented Oct 3, 2025

Uh oh!

llvmbot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kraj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

djtodoro Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrtc27 left a comment

Choose a reason for hiding this comment

Uh oh!

djtodoro commented Oct 7, 2025

Uh oh!

github-actions bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kraj commented Oct 8, 2025

Uh oh!

djtodoro commented Oct 8, 2025

Uh oh!

kraj left a comment

Choose a reason for hiding this comment

Uh oh!

kraj commented Oct 8, 2025

Uh oh!

djtodoro commented Oct 10, 2025

Uh oh!

brad0 commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

djtodoro Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ameisen commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

djtodoro commented Oct 31, 2025

Uh oh!

ameisen commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ameisen commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

ameisen left a comment

llvmbot commented Oct 3, 2025 •

edited

Loading

djtodoro Oct 7, 2025 •

edited

Loading

github-actions bot commented Oct 7, 2025 •

edited

Loading

djtodoro Oct 17, 2025 •

edited

Loading

ameisen commented Oct 30, 2025 •

edited

Loading

ameisen commented Oct 31, 2025 •

edited

Loading

ameisen commented Oct 31, 2025 •

edited

Loading