Fix passing/returning structs with the 64-bit SPARC ABI by beetrees · Pull Request #142680 · rust-lang/rust

beetrees · 2025-06-18T15:11:25Z

Fixes the 64-bit SPARC part of #115609 by replacing the current implementation with a new implementation modelled on the RISC-V calling convention code (SPARC ABI reference).

Pinging sparcv9-sun-solaris target maintainers: @psumbera @kulikjak
Fixes #115336
Fixes #115399
Fixes #122620
Fixes #147883
r? @workingjubilee

rustbot · 2025-06-18T15:11:29Z

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

beetrees · 2025-06-18T15:13:23Z

r? @bjorn3

beetrees · 2025-06-18T18:49:07Z

Note that there are currently two places that Clang and GCC disagree AFAIK:

Clang fails to correctly align 16-byte structs when passing them as arguments, whereas GCC correctly aligns them. As the specification states that the structs must be aligned, I've filed an Clang bug at Clang: 64-bit SPARC doesn't align struct arguments as required by ABI llvm/llvm-project#144709.
Clang passes floating point fields in packed structs in floating point register if the field is aligned within the argument as a whole, whereas GCC always passes packed structs in integer registers. The specification doesn't consider the existence of packed structs, however in my opinion Clang's interpretation seems more correct so that's what I've implemented in this PR. Issues for Rust, Clang and GCC.

psumbera · 2025-06-19T14:58:27Z

Thank you for doing this! My knowledge in this area is rather limited.

I was able to build latest Rust sources with your fix on top on Solaris 11.4 SPARC. Then I used this version to build Firefox on Solaris. There was no problem. Firefox also worked as expected.

I also tested some problematic cases which I was handling few years ago (https://github.com/psumbera/rust-sparc64-abi-tests). All seemed to be ok.

bjorn3 · 2025-07-03T11:23:02Z

I don't have time to properly review this right now.

r? compiler

bors · 2025-07-06T14:13:49Z

☔ The latest upstream changes (presumably #143521) made this pull request unmergeable. Please resolve the merge conflicts.

beetrees · 2025-07-06T14:45:16Z

Rebased

bors · 2025-07-22T22:48:52Z

☔ The latest upstream changes (presumably #144249) made this pull request unmergeable. Please resolve the merge conflicts.

wesleywiser · 2025-08-28T14:48:36Z

Hi @workingjubilee, are you still interested in reviewing this? If not, we can reroll for a different reviewer. Thanks!

tgross35 · 2025-09-05T02:17:41Z

Looks like there is a Clang fix in llvm/llvm-project#155829

tests/assembly-llvm/sparc-struct-abi.rs

compiler/rustc_target/src/callconv/sparc64.rs

tests/assembly-llvm/sparc-struct-abi.rs

compiler/rustc_target/src/callconv/sparc64.rs

workingjubilee · 2025-09-10T04:26:47Z

Sorry about that, currently in catchup mode so I am going to delegate, yes.

r? @tgross35

tgross35

This looks great, thanks for the updates. I requested a few more comments for things that weren't obvious, r=me after those.

View changes since this review

tgross35 · 2025-09-24T08:37:29Z

compiler/rustc_target/src/callconv/sparc64.rs

+    if let [Some(reg), None, ..] = regs {
+        arg.cast_to_and_pad_i32(CastTarget::from(reg).with_attrs(attrs.into()), pad);
+    } else {
+        arg.cast_to_and_pad_i32(
+            CastTarget::prefixed(regs, Uniform::new(Reg::i8(), Size::ZERO))
+                .with_attrs(attrs.into()),
+            pad,
+        );
+    }


Could you add a note about why the Some(reg), None case is special?

tgross35 · 2025-09-24T08:39:57Z

compiler/rustc_target/src/callconv/sparc64.rs

-        _ => {
-            if let BackendRepr::ScalarPair(scalar1, scalar2) = &layout.backend_repr {
-                data = arg_scalar_pair(cx, scalar1, scalar2, offset, data);
+            _ => {}


What happens in the fallback case where offset isn't aligned?

compiler/rustc_target/src/callconv/sparc64.rs

tgross35 · 2025-09-24T08:43:45Z

compiler/rustc_target/src/callconv/sparc64.rs

-                },
-                Size::ZERO,
-            );
+    let mut double_words = [DoubleWord::Words([Word::Integer; 2]); 4];


Could you note where the length 4 comes from, or put it in a const? Assuming 8 word-sized registers for arg passing.

compiler/rustc_target/src/callconv/sparc64.rs

tgross35 · 2025-09-24T08:50:43Z

@rustbot author
@bors delegate+

rustbot · 2025-09-24T08:50:46Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

bors · 2025-09-24T08:50:46Z

✌️ @beetrees, you can now approve this pull request!

If @tgross35 told you to "r=me" after making some further change, please make that change, then do @bors r=@tgross35

tgross35 · 2025-09-24T18:53:22Z

@psumbera if you have access to a sparc machine, any chance you could run https://github.com/Gankra/abi-cafe with and without this PR as a sanity check? You should be able to clone the repo and run cargo run -- --toolchains 'rustc,cc,gcc,clang' and post the list of failures from stdout. To rerun with the PATH=

It picks the first rustc/gcc/cc/clang so I think it needs to be run with PATH=$(realpath build/target/stage1/bin/rustc):$PATH to use a built version.

Edit: looks like packed structs may not work yet Gankra/abi-cafe#33. Still a valuable test._

brad0 · 2025-09-26T04:46:36Z

The LLVM fix llvm/llvm-project#155829 has gone into main and I am going to see about backporting to 21.

psumbera · 2025-09-26T06:12:52Z

@psumbera if you have access to a sparc machine, any chance you could run https://github.com/Gankra/abi-cafe with and without this PR as a sanity check? You should be able to clone the repo and run cargo run -- --toolchains 'rustc,cc,gcc,clang' and post the list of failures from stdout. To rerun with the PATH=

Not sure how to analyze it. But grep failed! test.log | wc -l gives me 156 without the change and 228 with the change. So there seems to be some regression.

Generally I would suggest to use for tests: https://portal.cfarm.net/machines/list/ (cfarm216). This is latest Solaris (SPARC-M8).

I'm ready to help and provide full logs somewhere or anything.

Here are just few tests which are newly failing:

+  F32Array::conv_c::repr_c::cc_calls_rustc::struct_in_1              failed!
+  F32Array::conv_c::repr_c::cc_calls_rustc::val_in                   failed!
+  F32Array::conv_c::repr_c::cc_calls_rustc::val_in_10                failed!
..
+  F32Array::conv_c::repr_c::gcc_calls_rustc::struct_in_1              failed!
+  F32Array::conv_c::repr_c::gcc_calls_rustc::val_in                   failed!
+  F32Array::conv_c::repr_c::gcc_calls_rustc::val_in_10                failed!
..
+  Test F32Array::conv_c::repr_c::cc_calls_rustc::struct_in_1              failed!
+  Test F32Array::conv_c::repr_c::cc_calls_rustc::val_in                   failed!
+  Test F32Array::conv_c::repr_c::cc_calls_rustc::val_in_10    
..
+  Test F32Array::conv_c::repr_c::gcc_calls_rustc::struct_in_1              failed!
+  Test F32Array::conv_c::repr_c::gcc_calls_rustc::val_in                   failed!
+  Test F32Array::conv_c::repr_c::gcc_calls_rustc::val_in_10                failed!
..

bors · 2025-09-29T12:32:51Z

☔ The latest upstream changes (presumably #147145) made this pull request unmergeable. Please resolve the merge conflicts.

brad0 · 2025-10-10T08:15:29Z

llvm/llvm-project#144709 has been backported to the 21 branch and has shipped with 21.1.3.
llvm/llvm-project#161766

folkertdev · 2025-10-20T21:12:55Z

Presumably this also fixes #147883, though it would be good to test for it explicitly.

We potentially also need to handle { i32, float } in a custom way. With clang/LLVM the struct fields are actually unpacked and passed separately. There some information on why here https://github.com/llvm/llvm-project/blob/fcb1a82811490ca581b4db484a2b67be86650a84/clang/lib/CodeGen/Targets/Sparc.cpp#L81-L106. However the current implementation (passing { i32, float } does appear to work as expected

https://godbolt.org/z/7sW5TE5Gx.

In any case, that probably deserves a test too given that LLVM calls it out as a special case.

SpriteOvO · 2026-01-16T12:22:46Z

Hi @beetrees, ping from triage team. Are there any further updates on this PR?

folkertdev · 2026-01-31T13:04:11Z

In @beetrees's absence, @tgross35 and I decided that we should move forward here to remove a blocker for f128, so I'll rebase and add some comments and this should be good to go.

rustbot · 2026-01-31T13:05:26Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

folkertdev · 2026-01-31T13:08:06Z

compiler/rustc_target/src/callconv/sparc64.rs

+    let cast_target = match regs {
+        [Some(reg), None, ..] => CastTarget::from(reg),
+        _ => CastTarget::prefixed(regs, Uniform::new(Reg::i8(), Size::ZERO)),
+    };


I'm not entirely sure why the special-casing is, but the first case fits in a register no problem, while the second case requires some sort of larger struct that is cast.

folkertdev · 2026-02-02T23:59:15Z

@rustbot ready

rustbot assigned workingjubilee Jun 18, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 18, 2025

rustbot assigned bjorn3 and unassigned workingjubilee Jun 18, 2025

tgross35 mentioned this pull request Jun 19, 2025

The sparc64, riscv64, loongarch64 extern "C" fn ABIs are all wrong when aligned/packed structs are involved #115609

Open

rustbot assigned WaffleLapkin and unassigned bjorn3 Jul 3, 2025

workingjubilee self-assigned this Jul 4, 2025

beetrees force-pushed the sparc64-float-struct-abi branch from 4eb3ce5 to 9e4a7bc Compare July 6, 2025 14:45

WaffleLapkin removed their assignment Jul 8, 2025

beetrees force-pushed the sparc64-float-struct-abi branch from 9e4a7bc to b7b3ed7 Compare July 27, 2025 08:50

tgross35 reviewed Sep 5, 2025

View reviewed changes

rustbot assigned tgross35 and unassigned workingjubilee Sep 10, 2025

beetrees force-pushed the sparc64-float-struct-abi branch from b7b3ed7 to 7cba46f Compare September 10, 2025 19:41

This comment has been minimized.

Sign in to view

beetrees force-pushed the sparc64-float-struct-abi branch from 7cba46f to b2e4ee9 Compare September 10, 2025 19:51

This comment has been minimized.

Sign in to view

tgross35 approved these changes Sep 24, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 24, 2025

folkertdev mentioned this pull request Oct 20, 2025

"Size::sub: 0 - 8 would result in negative size" ICE on sparc #147883

Open

folkertdev force-pushed the sparc64-float-struct-abi branch from b2e4ee9 to 7df3c25 Compare January 31, 2026 13:05

folkertdev reviewed Jan 31, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Fix passing/returning structs with the 64-bit SPARC ABI

7515aca

folkertdev force-pushed the sparc64-float-struct-abi branch from 7df3c25 to 7515aca Compare January 31, 2026 14:41

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 2, 2026

Uh oh!

Conversation

beetrees commented Jun 18, 2025 • edited by folkertdev Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 18, 2025

Uh oh!

beetrees commented Jun 18, 2025

Uh oh!

beetrees commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psumbera commented Jun 19, 2025

Uh oh!

bjorn3 commented Jul 3, 2025

Uh oh!

bors commented Jul 6, 2025

Uh oh!

beetrees commented Jul 6, 2025

Uh oh!

bors commented Jul 22, 2025

Uh oh!

wesleywiser commented Aug 28, 2025

Uh oh!

tgross35 commented Sep 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

workingjubilee commented Sep 10, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

tgross35 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tgross35 Sep 24, 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!

Uh oh!

Uh oh!

tgross35 commented Sep 24, 2025

Uh oh!

rustbot commented Sep 24, 2025

Uh oh!

bors commented Sep 24, 2025

Uh oh!

tgross35 commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brad0 commented Sep 26, 2025

Uh oh!

psumbera commented Sep 26, 2025

Uh oh!

bors commented Sep 29, 2025

Uh oh!

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

Uh oh!

Uh oh!

folkertdev commented Oct 20, 2025

Uh oh!

SpriteOvO commented Jan 16, 2026

Uh oh!

folkertdev commented Jan 31, 2026

Uh oh!

rustbot commented Jan 31, 2026

Uh oh!

folkertdev Jan 31, 2026

beetrees commented Jun 18, 2025 •

edited by folkertdev

Loading

beetrees commented Jun 18, 2025 •

edited

Loading

tgross35 left a comment •

edited by rustbot

Loading

tgross35 Sep 24, 2025 •

edited

Loading

tgross35 commented Sep 24, 2025 •

edited

Loading

brad0 commented Oct 10, 2025 •

edited

Loading