[LoongArch][RISCV] Fix incorrect indexing of incoming byval arguments in tail call eligibility check#188006
Merged
[LoongArch][RISCV] Fix incorrect indexing of incoming byval arguments in tail call eligibility check#188006
Conversation
… in tail call eligibility check
Member
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-backend-loongarch Author: hev (heiher) ChangesThe loop that validates byval arguments in This mismatch could lead to out-of-bounds access or incorrect type comparisons when non-byval arguments are interleaved with byval arguments, causing the tail call eligibility check to fail or behave incorrectly. Fix this by using This issue affects both LoongArch and RISCV backends, which share the same logic pattern. Fixes #187832 4 Files Affected:
diff --git a/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp b/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
index 9deda5ca564b3..7a0996ce9a43d 100644
--- a/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
@@ -8565,9 +8565,9 @@ bool LoongArchTargetLowering::isEligibleForTailCallOptimization(
for (unsigned i = 0, j = 0; i < Outs.size(); i++) {
if (!Outs[i].Flags.isByVal())
continue;
- if (j++ >= LoongArchFI->getIncomingByValArgsSize())
+ if (j >= LoongArchFI->getIncomingByValArgsSize())
return false;
- if (LoongArchFI->getIncomingByValArgs(i).getValueType() != Outs[i].ArgVT)
+ if (LoongArchFI->getIncomingByValArgs(j++).getValueType() != Outs[i].ArgVT)
return false;
}
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 7c1eacbce3701..c277b1e6c8dad 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -24752,9 +24752,9 @@ bool RISCVTargetLowering::isEligibleForTailCallOptimization(
for (unsigned i = 0, j = 0, e = Outs.size(); i != e; ++i) {
if (!Outs[i].Flags.isByVal())
continue;
- if (j++ >= RVFI->getIncomingByValArgsSize())
+ if (j >= RVFI->getIncomingByValArgsSize())
return false;
- if (RVFI->getIncomingByValArgs(i).getValueType() != Outs[i].ArgVT)
+ if (RVFI->getIncomingByValArgs(j++).getValueType() != Outs[i].ArgVT)
return false;
}
diff --git a/llvm/test/CodeGen/LoongArch/issue187832.ll b/llvm/test/CodeGen/LoongArch/issue187832.ll
new file mode 100644
index 0000000000000..b483a7640e171
--- /dev/null
+++ b/llvm/test/CodeGen/LoongArch/issue187832.ll
@@ -0,0 +1,48 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=loongarch32 %s -o - | FileCheck %s --check-prefix=LA32
+; RUN: llc -mtriple=loongarch64 %s -o - | FileCheck %s --check-prefix=LA64
+
+%Box = type { i32, i32, i32, i8, [3 x i8], i32, i8, [1 x i8], i16, i16, i8, [5 x i8], { i64, ptr }, { i64, ptr }, { i64, ptr } }
+
+define void @test(ptr byval(%Box) %0) nounwind {
+; LA32-LABEL: test:
+; LA32: # %bb.0:
+; LA32-NEXT: addi.w $sp, $sp, -112
+; LA32-NEXT: st.w $ra, $sp, 108 # 4-byte Folded Spill
+; LA32-NEXT: st.w $fp, $sp, 104 # 4-byte Folded Spill
+; LA32-NEXT: addi.w $fp, $sp, 24
+; LA32-NEXT: ori $a2, $zero, 80
+; LA32-NEXT: move $a0, $fp
+; LA32-NEXT: move $a1, $zero
+; LA32-NEXT: bl memcpy
+; LA32-NEXT: st.w $zero, $sp, 8
+; LA32-NEXT: st.w $zero, $sp, 4
+; LA32-NEXT: st.w $zero, $sp, 0
+; LA32-NEXT: move $a0, $zero
+; LA32-NEXT: move $a1, $zero
+; LA32-NEXT: move $a2, $zero
+; LA32-NEXT: move $a3, $fp
+; LA32-NEXT: move $a4, $zero
+; LA32-NEXT: move $a5, $zero
+; LA32-NEXT: move $a6, $zero
+; LA32-NEXT: move $a7, $zero
+; LA32-NEXT: jirl $ra, $zero, 0
+; LA32-NEXT: ld.w $fp, $sp, 104 # 4-byte Folded Reload
+; LA32-NEXT: ld.w $ra, $sp, 108 # 4-byte Folded Reload
+; LA32-NEXT: addi.w $sp, $sp, 112
+; LA32-NEXT: ret
+;
+; LA64-LABEL: test:
+; LA64: # %bb.0:
+; LA64-NEXT: movgr2fr.d $fa0, $zero
+; LA64-NEXT: move $a0, $zero
+; LA64-NEXT: move $a1, $zero
+; LA64-NEXT: move $a2, $zero
+; LA64-NEXT: move $a3, $zero
+; LA64-NEXT: move $a4, $zero
+; LA64-NEXT: move $a5, $zero
+; LA64-NEXT: move $a6, $zero
+; LA64-NEXT: jr $a0
+ tail call void null(ptr null, double 0.000000e+00, ptr byval(%Box) null, { i64, ptr } zeroinitializer, i32 0, i64 0, i1 false)
+ ret void
+}
diff --git a/llvm/test/CodeGen/RISCV/issue187832.ll b/llvm/test/CodeGen/RISCV/issue187832.ll
new file mode 100644
index 0000000000000..dd4c3c6e3487e
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/issue187832.ll
@@ -0,0 +1,48 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=riscv32 %s -o - | FileCheck %s --check-prefix=RV32
+; RUN: llc -mtriple=riscv64 %s -o - | FileCheck %s --check-prefix=RV64
+
+%Box = type { i32, i32, i32, i8, [3 x i8], i32, i8, [1 x i8], i16, i16, i8, [5 x i8], { i64, ptr }, { i64, ptr }, { i64, ptr } }
+
+define void @test(ptr byval(%Box) %0) nounwind {
+; RV32-LABEL: test:
+; RV32: # %bb.0:
+; RV32-NEXT: addi sp, sp, -112
+; RV32-NEXT: sw ra, 108(sp) # 4-byte Folded Spill
+; RV32-NEXT: sw s0, 104(sp) # 4-byte Folded Spill
+; RV32-NEXT: addi a0, sp, 24
+; RV32-NEXT: li a2, 80
+; RV32-NEXT: li s0, 0
+; RV32-NEXT: li a1, 0
+; RV32-NEXT: call memcpy
+; RV32-NEXT: addi a3, sp, 24
+; RV32-NEXT: sw zero, 0(sp)
+; RV32-NEXT: sw zero, 4(sp)
+; RV32-NEXT: sw zero, 8(sp)
+; RV32-NEXT: li a0, 0
+; RV32-NEXT: li a1, 0
+; RV32-NEXT: li a2, 0
+; RV32-NEXT: li a4, 0
+; RV32-NEXT: li a5, 0
+; RV32-NEXT: li a6, 0
+; RV32-NEXT: li a7, 0
+; RV32-NEXT: jalr s0
+; RV32-NEXT: lw ra, 108(sp) # 4-byte Folded Reload
+; RV32-NEXT: lw s0, 104(sp) # 4-byte Folded Reload
+; RV32-NEXT: addi sp, sp, 112
+; RV32-NEXT: ret
+;
+; RV64-LABEL: test:
+; RV64: # %bb.0:
+; RV64-NEXT: li a0, 0
+; RV64-NEXT: li a1, 0
+; RV64-NEXT: li a2, 0
+; RV64-NEXT: li a3, 0
+; RV64-NEXT: li a4, 0
+; RV64-NEXT: li a5, 0
+; RV64-NEXT: li a6, 0
+; RV64-NEXT: li a7, 0
+; RV64-NEXT: jr a0
+ tail call void null(ptr null, double 0.000000e+00, ptr byval(%Box) null, { i64, ptr } zeroinitializer, i32 0, i64 0, i1 false)
+ ret void
+}
|
topperc
reviewed
Mar 23, 2026
lenary
reviewed
Mar 23, 2026
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/43993 Here is the relevant piece of the build log for the reference |
heiher
added a commit
that referenced
this pull request
Mar 26, 2026
…l call eligibility check (#188006) The loop that validates byval arguments in `isEligibleForTailCallOptimization()` incorrectly used the loop index `i` when accessing `getIncomingByValArgs()`, even though `j` is the index tracking the number of encountered byval arguments. This mismatch could lead to out-of-bounds access or incorrect type comparisons when non-byval arguments are interleaved with byval arguments, causing the tail call eligibility check to fail or behave incorrectly. Fix this by using `j` consistently as the index into the incoming byval argument list and only incrementing it after the bounds check. Fixes #187832 (cherry picked from commit ab17b54)
dyung
pushed a commit
that referenced
this pull request
Mar 27, 2026
…l call eligibility check (#188006) The loop that validates byval arguments in `isEligibleForTailCallOptimization()` incorrectly used the loop index `i` when accessing `getIncomingByValArgs()`, even though `j` is the index tracking the number of encountered byval arguments. This mismatch could lead to out-of-bounds access or incorrect type comparisons when non-byval arguments are interleaved with byval arguments, causing the tail call eligibility check to fail or behave incorrectly. Fix this by using `j` consistently as the index into the incoming byval argument list and only incrementing it after the bounds check. Fixes #187832 (cherry picked from commit ab17b54)
Aadarsh-Keshri
pushed a commit
to Aadarsh-Keshri/llvm-project
that referenced
this pull request
Mar 28, 2026
… in tail call eligibility check (llvm#188006) The loop that validates byval arguments in `isEligibleForTailCallOptimization()` incorrectly used the loop index `i` when accessing `getIncomingByValArgs()`, even though `j` is the index tracking the number of encountered byval arguments. This mismatch could lead to out-of-bounds access or incorrect type comparisons when non-byval arguments are interleaved with byval arguments, causing the tail call eligibility check to fail or behave incorrectly. Fix this by using `j` consistently as the index into the incoming byval argument list and only incrementing it after the bounds check. This issue affects both LoongArch and RISCV backends, which share the same logic pattern. Fixes llvm#187832
lenary
added a commit
to lenary/llvm-project
that referenced
this pull request
Apr 10, 2026
This reverts: - 2b839f6 (llvm#168506) - 6a81656 (llvm#170547) - ab17b54 (llvm#188006) - e65dd1f (llvm#191093) The changes in llvm#168506 and llvm#170547 both have a lifetime issue where an SDValue is kept for the duration of a function, despite being valid only when processing the same basic block. Reverting both on LoongArch and RISC-V as the implementations are identical and one of the fix commits touches both targets, rather than doing only a RISC-V revert. I also think this more cleanly shows what is being undone when starting again with the changes.
lenary
added a commit
that referenced
this pull request
Apr 10, 2026
This reverts: - 2b839f6 (#168506) - 6a81656 (#170547) - ab17b54 (#188006) - e65dd1f (#191093) The changes in #168506 and #170547 both have a lifetime issue where an SDValue is kept for the duration of a function, despite being valid only when processing the same basic block. Reverting both on LoongArch and RISC-V as the implementations are identical and one of the fix commits touches both targets, rather than doing only a RISC-V revert. I also think this more cleanly shows what is being undone when starting again with the changes.
lenary
added a commit
to lenary/llvm-project
that referenced
this pull request
Apr 10, 2026
This reverts: - 2b839f6 (llvm#168506) - d40e607 (llvm#188006) There's a lifetime issue in the implementation, where an SDValue is saved and may be used outside the current basic block. The corresponding revert on `main` is 501417b (llvm#191508) - in this case only the LoongArch changes made it to the 22.x branches, so this commit only affects that architecture.
c-rhodes
pushed a commit
to lenary/llvm-project
that referenced
this pull request
Apr 17, 2026
This reverts: - 2b839f6 (llvm#168506) - d40e607 (llvm#188006) There's a lifetime issue in the implementation, where an SDValue is saved and may be used outside the current basic block. The corresponding revert on `main` is 501417b (llvm#191508) - in this case only the LoongArch changes made it to the 22.x branches, so this commit only affects that architecture.
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
6 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.
The loop that validates byval arguments in
isEligibleForTailCallOptimization()incorrectly used the loop indexiwhen accessinggetIncomingByValArgs(), even thoughjis the index tracking the number of encountered byval arguments.This mismatch could lead to out-of-bounds access or incorrect type comparisons when non-byval arguments are interleaved with byval arguments, causing the tail call eligibility check to fail or behave incorrectly.
Fix this by using
jconsistently as the index into the incoming byval argument list and only incrementing it after the bounds check.This issue affects both LoongArch and RISCV backends, which share the same logic pattern.
Fixes #187832