Skip to content

Add assembly test for -Zreg-struct-return option #145382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add assembly test for -Zreg-struct-return option #145382

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d06c2d4
Add test file for reg-struct-return
winstonallo Aug 13, 2025
24a767d
Add assembly tests for `-Zreg-struct-return`
winstonallo Aug 14, 2025
fc67ace
Improve test description in comment
winstonallo Aug 14, 2025
427a183
Add better test description
winstonallo Aug 14, 2025
f4a9317
Add tests for < 8 and > 8 bytes structs
winstonallo Aug 14, 2025
3518ac1
Simplify WITHOUT test in pivot_call
winstonallo Aug 14, 2025
fcf55d5
Generalize target registers in tests
winstonallo Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions tests/assembly-llvm/reg-struct-return.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Tests that -Zreg-struct-return changes ABI for small struct returns
// from hidden-pointer convention to register-return convention on x86.
//@ add-core-stubs
//@ assembly-output: emit-asm
//@ compile-flags: -O --target=i686-unknown-linux-gnu -Crelocation-model=static
//@ revisions: WITH WITHOUT
//@[WITH] compile-flags: -Zreg-struct-return
//@ needs-llvm-components: x86

#![feature(no_core)]
#![no_std]
#![no_core]
#![crate_type = "lib"]

extern crate minicore;
use minicore::*;

struct div_t {
quot: i32,
rem: i32,
}

unsafe extern "C" {
fn div(numerator: i32, denominator: i32) -> div_t;
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn direct_construction(numerator: i32, denominator: i32) -> div_t {
// WITH-LABEL: direct_construction
// WITH: movl $42, %eax
// WITH: movl $42, %edx
// WITH: retl

// WITHOUT-LABEL: direct_construction
// WITHOUT: movl 4(%esp), %eax
// WITHOUT: movl $42, (%eax)
// WITHOUT: movl $42, 4(%eax)
// WITHOUT: retl $4
div_t { quot: 42, rem: 42 }
}

#[unsafe(no_mangle)]
pub unsafe extern "C" fn builtin_call(numerator: i32, denominator: i32) -> div_t {
// WITH-LABEL: builtin_call
// WITH: jmp div

// WITHOUT-LABEL: builtin_call
// WITHOUT: pushl %esi
// WITHOUT: subl $8, %esp
// WITHOUT: movl 16(%esp), %esi
// WITHOUT: subl $4, %esp
// WITHOUT: pushl 28(%esp)
// WITHOUT: pushl 28(%esp)
// WITHOUT: pushl %esi
// WITHOUT: calll div
// WITHOUT: addl $12, %esp
// WITHOUT: movl %esi, %eax
// WITHOUT: addl $8, %esp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I am not sure how strict we want to be matching the entire stream of instructions for this case -- it could be improved or shuffled a bit by the backend.

On the other hand, this may be easy to relax if a newer LLVM changes it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I was also wondering, it's definitely easy to relax later but would be nice to have a maintainer's opinion on this to make sure this test is not flaky. I am unfortunately not very familiar with how often LLVM changes this kind of stuff

Copy link
Author

@winstonallo winstonallo Aug 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it would be fine to limit this to what differentiates it from the reg-struct-return assembly, so the calll and the retl $4, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, like in the new ones you just added, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the code here was:

pub unsafe extern "C" fn builtin_call(numerator: i32, denominator: i32) -> div_t {
    // WITH-LABEL: builtin_call
    // WITH: jmp div
    // WITHOUT-LABEL: builtin_call
    // WITHOUT: pushl %esi
    // WITHOUT: subl $8, %esp
    // WITHOUT: movl 16(%esp), %esi
    // WITHOUT: subl $4, %esp
    // WITHOUT: pushl 28(%esp)
    // WITHOUT: pushl 28(%esp)
    // WITHOUT: pushl %esi
    // WITHOUT: calll div
    // WITHOUT: addl $12, %esp
    // WITHOUT: movl %esi, %eax
    // WITHOUT: addl $8, %esp
    // WITHOUT: popl %esi
    // WITHOUT: retl $4
    div(numerator, denominator)
}

Which doesn't exist anymore. For reference though, here's a way-too-detailed answer:

The above is testing a lot: both the caller side and the callee side, as well as how i32 gets passed. For ABI tests it's best to make each test function isolate one of those aspects, which gives you less to assert on. For checking returns usually that means returning some immediate value to test the callee side (like you have now), and then storing the result of a call (#145382 (comment)) to test the caller side. Then you basically only need to check that something gets written to the place you expect and can be more lax about how it gets there.

LLVM's repo has a lot more assertions about exact instruction sequences (using -NEXT), but these are usually autogenerated and run without optimizations so it's not as likely to be volatile. An example that's fresh in my head: https://github.com/llvm/llvm-project/blob/c22ec9cde3708e0c7afd0909508a67ef9625aa4c/llvm/test/CodeGen/X86/i128-fp128-abi.ll. We tend to avoid such tests in this repo because it's more painful to update when things do change, and LLVM covers ABI things pretty well.

For CHECK: directives where the ordering doesn't matter, FileCheck has CHECK-DAG. No need really to do this here, though it doesn't hurt if you feel like it (might make it possible to use the same test for the clif and gcc backends once/if those start getting asm tests).

It also has a cool feature where you can bind a regex to a name and then use it later, useful if you know it will use a scratch register but don't care which https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive.

// WITHOUT: popl %esi
// WITHOUT: retl $4
div(numerator, denominator)
}
Loading