[SPARC] Do not use the unimp struct-return convention for zero-sized types - #213733
Merged
Conversation
folkertdev
commented
Aug 3, 2026
Comment on lines
+319
to
+337
| // return to skip over. | ||
| Type *RetType = MF.getFunction().getParamStructRetType(0); | ||
| if (!RetType->isEmptyTy()) | ||
| RetAddrOffset = 12; // CallInst + Delay Slot + Unimp |
Contributor
Author
There was a problem hiding this comment.
the gcc logic for picking 8 or 12 is at
|
@llvm/pr-subscribers-backend-sparc Author: Folkert de Vries (folkertdev) ChangesGCC has special handling for an https://godbolt.org/z/Tcraao7rT It seems unlikely someone is actually relying on this, so I haven't added the abi compatibility logic. Rust however uses zero-sized types quite heavily, so having this work correctly is useful there. Full diff: https://github.com/llvm/llvm-project/pull/213733.diff 3 Files Affected:
diff --git a/llvm/lib/Target/Sparc/DelaySlotFiller.cpp b/llvm/lib/Target/Sparc/DelaySlotFiller.cpp
index 7d7aa20869c20..03ca5e7ae98d2 100644
--- a/llvm/lib/Target/Sparc/DelaySlotFiller.cpp
+++ b/llvm/lib/Target/Sparc/DelaySlotFiller.cpp
@@ -391,8 +391,12 @@ bool Filler::needsUnimp(MachineBasicBlock::iterator I, unsigned &StructSize)
const MachineOperand &MO = I->getOperand(structSizeOpNum);
if (!MO.isImm())
return false;
+
+ // A zero-sized return value has nothing for the callee to copy, so GCC emits
+ // no unimp for it and returns to the instruction right after the delay slot.
+ // We replicate this behavior here.
StructSize = MO.getImm();
- return true;
+ return StructSize != 0;
}
static bool combineRestoreADD(MachineBasicBlock &MBB,
diff --git a/llvm/lib/Target/Sparc/SparcISelLowering.cpp b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
index 488cc2f8e900f..f705c95d8531c 100644
--- a/llvm/lib/Target/Sparc/SparcISelLowering.cpp
+++ b/llvm/lib/Target/Sparc/SparcISelLowering.cpp
@@ -328,7 +328,13 @@ SparcTargetLowering::LowerReturn_32(SDValue Chain, CallingConv::ID CallConv,
Chain = DAG.getCopyToReg(Chain, DL, SP::I0, Val, Glue);
Glue = Chain.getValue(1);
RetOps.push_back(DAG.getRegister(SP::I0, PtrVT));
- RetAddrOffset = 12; // CallInst + Delay Slot + Unimp
+
+ // A zero-sized return value, e.g. an empty struct or union, is returned
+ // without an unimp instruction after the call, so there is nothing for the
+ // return to skip over.
+ Type *RetType = MF.getFunction().getParamStructRetType(0);
+ if (!RetType->isEmptyTy())
+ RetAddrOffset = 12; // CallInst + Delay Slot + Unimp
}
RetOps[0] = Chain; // Update chain.
diff --git a/llvm/test/CodeGen/SPARC/2011-01-22-SRet.ll b/llvm/test/CodeGen/SPARC/2011-01-22-SRet.ll
index d8a8caf0d83f5..0bdec29de39df 100644
--- a/llvm/test/CodeGen/SPARC/2011-01-22-SRet.ll
+++ b/llvm/test/CodeGen/SPARC/2011-01-22-SRet.ll
@@ -1,12 +1,21 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
;RUN: llc -mtriple=sparc < %s | FileCheck %s
%struct.foo_t = type { i32, i32, i32 }
+%struct.empty_t = type { { } }
+%struct.emptyarr_t = type { [0 x i32] }
+%struct.tiny_t = type { i8 }
define weak void @make_foo(ptr noalias sret(%struct.foo_t) %agg.result, i32 %a, i32 %b, i32 %c) nounwind {
+; CHECK-LABEL: make_foo:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: ld [%sp+64], %o3
+; CHECK-NEXT: st %o0, [%o3]
+; CHECK-NEXT: st %o1, [%o3+4]
+; CHECK-NEXT: st %o2, [%o3+8]
+; CHECK-NEXT: jmp %o7+12
+; CHECK-NEXT: mov %o3, %o0
entry:
-;CHECK-LABEL: make_foo:
-;CHECK: ld [%sp+64], {{.+}}
-;CHECK: jmp %o7+12
%0 = getelementptr inbounds %struct.foo_t, ptr %agg.result, i32 0, i32 0
store i32 %a, ptr %0, align 4
%1 = getelementptr inbounds %struct.foo_t, ptr %agg.result, i32 0, i32 1
@@ -17,11 +26,24 @@ entry:
}
define i32 @test() nounwind {
+; CHECK-LABEL: test:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: save %sp, -112, %sp
+; CHECK-NEXT: add %fp, -16, %i0
+; CHECK-NEXT: mov 10, %o0
+; CHECK-NEXT: mov 20, %o1
+; CHECK-NEXT: mov 30, %o2
+; CHECK-NEXT: call make_foo
+; CHECK-NEXT: st %i0, [%sp+64]
+; CHECK-NEXT: unimp 12
+; CHECK-NEXT: ld [%fp+-16], %i1
+; CHECK-NEXT: or %i0, 4, %i0
+; CHECK-NEXT: ld [%i0], %i0
+; CHECK-NEXT: ld [%fp+-8], %i2
+; CHECK-NEXT: add %i0, %i1, %i0
+; CHECK-NEXT: ret
+; CHECK-NEXT: restore %i0, %i2, %o0
entry:
-;CHECK-LABEL: test:
-;CHECK: call make_foo
-;CHECK: st {{.+}}, [%sp+64]
-;CHECK: unimp 12
%f = alloca %struct.foo_t, align 8
call void @make_foo(ptr noalias sret(%struct.foo_t) %f, i32 10, i32 20, i32 30) nounwind
%0 = getelementptr inbounds %struct.foo_t, ptr %f, i32 0, i32 0
@@ -34,3 +56,82 @@ entry:
%7 = add nsw i32 %6, %5
ret i32 %7
}
+
+define weak void @make_tiny(ptr noalias sret(%struct.tiny_t) %agg.result) nounwind {
+; CHECK-LABEL: make_tiny:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: ld [%sp+64], %o0
+; CHECK-NEXT: mov 1, %o1
+; CHECK-NEXT: jmp %o7+12
+; CHECK-NEXT: stb %o1, [%o0]
+entry:
+ store i8 1, ptr %agg.result
+ ret void
+}
+
+define i32 @test_tiny() nounwind {
+; CHECK-LABEL: test_tiny:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: save %sp, -96, %sp
+; CHECK-NEXT: add %fp, -1, %i0
+; CHECK-NEXT: call make_tiny
+; CHECK-NEXT: st %i0, [%sp+64]
+; CHECK-NEXT: unimp 1
+; CHECK-NEXT: ret
+; CHECK-NEXT: restore %g0, %g0, %o0
+entry:
+ %e = alloca %struct.tiny_t, align 1
+ call void @make_tiny(ptr noalias sret(%struct.tiny_t) %e) nounwind
+ ret i32 0
+}
+
+; A zero-sized struct has nothing for the callee to copy, so the caller emits no
+; unimp after the call and the callee returns to %o7+8, which is spelled `retl`.
+define weak void @make_empty(ptr noalias sret(%struct.empty_t) %agg.result) nounwind {
+; CHECK-LABEL: make_empty:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: retl
+; CHECK-NEXT: ld [%sp+64], %o0
+entry:
+ ret void
+}
+
+define i32 @test_empty() nounwind {
+; CHECK-LABEL: test_empty:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: save %sp, -96, %sp
+; CHECK-NEXT: add %fp, -1, %i0
+; CHECK-NEXT: call make_empty
+; CHECK-NEXT: st %i0, [%sp+64]
+; CHECK-NEXT: ret
+; CHECK-NEXT: restore %g0, %g0, %o0
+entry:
+ %e = alloca %struct.empty_t, align 1
+ call void @make_empty(ptr noalias sret(%struct.empty_t) %e) nounwind
+ ret i32 0
+}
+
+; Test with empty arrays too.
+define weak void @make_emptyarr(ptr noalias sret(%struct.emptyarr_t) %agg.result) nounwind {
+; CHECK-LABEL: make_emptyarr:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: retl
+; CHECK-NEXT: ld [%sp+64], %o0
+entry:
+ ret void
+}
+
+define i32 @test_emptyarr() nounwind {
+; CHECK-LABEL: test_emptyarr:
+; CHECK: ! %bb.0: ! %entry
+; CHECK-NEXT: save %sp, -96, %sp
+; CHECK-NEXT: add %fp, -4, %i0
+; CHECK-NEXT: call make_emptyarr
+; CHECK-NEXT: st %i0, [%sp+64]
+; CHECK-NEXT: ret
+; CHECK-NEXT: restore %g0, %g0, %o0
+entry:
+ %e = alloca %struct.emptyarr_t, align 4
+ call void @make_emptyarr(ptr noalias sret(%struct.emptyarr_t) %e) nounwind
+ ret i32 0
+}
|
s-barannikov
reviewed
Aug 3, 2026
s-barannikov
left a comment
s-barannikov
left a comment
Contributor
There was a problem hiding this comment.
Looks reasonable, but I'd like @koachan to have eyes on this.
koachan
approved these changes
Aug 4, 2026
koachan
left a comment
koachan
left a comment
Contributor
There was a problem hiding this comment.
The code itself looks okay to me too, but, wow, what a surprising behavior of GCC 🥴
But given that we want compatibility with GCC, then I'd say it's all good~
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
3 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.
GCC has special handling for an
sretof a zero-sized type, match that handling.https://godbolt.org/z/Tcraao7rT
It seems unlikely someone is actually relying on this, so I haven't added the abi compatibility logic. Rust however uses zero-sized types quite heavily, so having this work correctly is useful there.