Skip to content

support -Zmin-function-alignment #1572

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.

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

Merged
merged 1 commit into from
Apr 21, 2025

Conversation

folkertdev
Copy link
Contributor

fixes #1555

Based on the timing, this should be supported now

I strongly suspect that no tests will hit this though (though I don't see how the relevant tests are ignored in scripts/test_rustc_tests.sh). As far as I can tell, function alignment with #[repr(align(N))] is not yet supported. I vaguely rember we discussed how this could be added, but now I can't find a way in the cranelift docs to align an individual function.

@bjorn3
Copy link
Member

bjorn3 commented Apr 20, 2025

The Cranelift PR got merged after the current release branched. It will be in the next Cranelift release, which will be released somewhere ij the next couple of days.

There is a release every month and two weeks before the release, a new branch is created for it to let the changes bake a bit and have bugfixes backported without introducing new regressions.

@folkertdev
Copy link
Contributor Author

Ah, well the next release should be soon then. But also, that this passed CI means that the logic never actually gets hit right? So the relevant tests are ignored by something in scripts/test_rustc_tests.sh?

@bjorn3
Copy link
Member

bjorn3 commented Apr 20, 2025

The only tests for this are codegen tests, which don't get run by test_rustc_tests.sh as --emit asm is not supported by cg_clif and in general those tests are too specific in what instructions they match to pass with any backend other than the LLVM backend anyway.

@bjorn3
Copy link
Member

bjorn3 commented Apr 21, 2025

The master branch is now updated to the new Cranelift release.

@folkertdev folkertdev force-pushed the min-function-alignment branch from 35dba80 to 342a75e Compare April 21, 2025 16:52
@folkertdev
Copy link
Contributor Author

I've rebased, with that:

  • should we do anything in terms of testing now?
  • do you have pointers for how to support #[repr(align(N))] on functions? We can then add some UI tests for the flag and the attribute, just to assert that the address in practice is actually a multiple of the specified alignment.

@bjorn3
Copy link
Member

bjorn3 commented Apr 21, 2025

do you have pointers for how to support #[repr(align(N))] on functions?

That can't be implemented yet until cranelift-module supports specifying a TargetIsa for individual function.

should we do anything in terms of testing now?

If you have locally checked that it sets the correct alignment in the function sections, that is enough as far as I'm concerned.

@folkertdev
Copy link
Contributor Author

I ran this, so I think we're good

> cat src/main.rs
#![feature(fn_align)]

#[repr(align(512))]
#[no_mangle]
fn foo() {}

#[repr(align(512))]
fn main() {
    assert!((foo as usize).is_multiple_of(512));
    assert!((main as usize).is_multiple_of(512));
}

> RUSTFLAGS="-Zmin-function-alignment=512" ~/rust/rustc_codegen_cranelift/dist/cargo-clif run
   Compiling playground v0.1.0 (/home/folkertdev/rust/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/playground`
> RUSTFLAGS="-Zmin-function-alignment=256" ~/rust/rustc_codegen_cranelift/dist/cargo-clif run
   Compiling playground v0.1.0 (/home/folkertdev/rust/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/playground`

thread 'main' panicked at src/main.rs:10:5:
assertion failed: (main as usize).is_multiple_of(512)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fish: Job 1, 'RUSTFLAGS="-Zmin-function-align…' terminated by signal SIGABRT (Abort)

@bjorn3 bjorn3 merged commit 0103c58 into rust-lang:master Apr 21, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support -Zmin-function-alignment
2 participants