[Mips] Fix getInstSizeInBytes for instructions with delay slots - #216665
Merged
Conversation
|
@llvm/pr-subscribers-backend-mips Author: yingopq ChangesMIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions. This issue was exposed in llvm 23 by commit pr #191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug. Thanks for the pr #187703 Fix #112010. Full diff: https://github.com/llvm/llvm-project/pull/216665.diff 1 Files Affected:
diff --git a/llvm/lib/Target/Mips/MipsInstrInfo.cpp b/llvm/lib/Target/Mips/MipsInstrInfo.cpp
index 8648aa0836693..446d02ebfc4d8 100644
--- a/llvm/lib/Target/Mips/MipsInstrInfo.cpp
+++ b/llvm/lib/Target/Mips/MipsInstrInfo.cpp
@@ -708,6 +708,10 @@ bool MipsInstrInfo::isAsCheapAsAMove(const MachineInstr &MI) const {
unsigned MipsInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
switch (MI.getOpcode()) {
default:
+ if (MI.hasDelaySlot()) {
+ // instr + 1 nop
+ return 8;
+ }
return MI.getDesc().getSize();
case TargetOpcode::INLINEASM:
case TargetOpcode::INLINEASM_BR: { // Inline Asm: Variable size.
|
Member
|
I rebuilt LLVM 23.1.0-rc3 with this patch, reverted https://codeberg.org/ziglang/zig/commit/4223910ab69147b2deb63aed38f40888a6977bc2, and can confirm that Zig's module tests pass for all MIPS targets again. Let's make sure this is backported to 23.x. |
Contributor
Author
|
@nikic Can you help review? |
nikic
reviewed
Aug 18, 2026
| default: | ||
| if (MI.hasDelaySlot()) { | ||
| // instr + 1 nop | ||
| return 8; |
Contributor
There was a problem hiding this comment.
Can you make this an addition on top of MI.getDesc().getSize() instead?
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
MIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions. This issue was exposed in llvm 23 by commit pr#191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug. Thanks for the pr#187703 `AllowOverEstimate` to help find instr which actual size mismatch expected size . Fix llvm#112010.
yingopq
force-pushed
the
Fix_bug_issue_112010
branch
from
August 19, 2026 07:52
3e7468e to
365e9c3
Compare
github-project-automation
Bot
moved this from Needs Backport PR
to Done
in LLVM Release Status
dyung
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 20, 2026
…#216665) MIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions. This issue was exposed in llvm 23 by commit pr llvm#191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug. Thanks for the pr llvm#187703 `AllowOverEstimate` to help find instr which actual size mismatch expected size . Fix llvm#112010. (cherry picked from commit a97f512)
kieroxide
pushed a commit
to kieroxide/llvm-project
that referenced
this pull request
Aug 21, 2026
…#216665) MIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions. This issue was exposed in llvm 23 by commit pr llvm#191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug. Thanks for the pr llvm#187703 `AllowOverEstimate` to help find instr which actual size mismatch expected size . Fix llvm#112010.
FlyGoat
added a commit
to FlyGoat/llvm-project
that referenced
this pull request
Aug 24, 2026
Follow-up to llvm#216665. Count complete instruction bundles so long-branch expansion includes both delay-slot and forbidden-slot instructions when estimating offsets.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MIPS branch/jump instructions (B, BEQ, JALR64Pseudo, PseudoReturn64, etc.) have a delay slot. The actual encoded size is 8 bytes (instr + NOP). This fixes "out of range PC16 fixup" errors on large functions.
This issue was exposed in llvm 23 by commit pr #191460 which changed MipsBranchExpansion to use MBB::iterator instead of instr_iterator, making the MBB size calculation more accurate and revealing the pre-existing bug.
Thanks for the pr #187703
AllowOverEstimateto help find instr which actual size mismatch expected size .Fix #112010.