Skip to content
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

CI updates #760

Merged
merged 3 commits into from
Feb 19, 2025
Merged

CI updates #760

merged 3 commits into from
Feb 19, 2025

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Feb 18, 2025

See the commit log for details.

Github has deprecated v2 so this needs to be bumped.
Pin i686-pc-windows-gnu to nightly-2025-02-07 until [1] is resolved.

[1]: rust-lang/rust#136795
@tgross35 tgross35 changed the title Ci updates CI updates Feb 18, 2025
@tgross35
Copy link
Contributor Author

Looks like something may have changed with aarch64. The last good test run was with rustc 1.86.0-nightly (a9730c3b5 2025-02-05), here https://github.com/rust-lang/compiler-builtins/actions/runs/13190067647/job/36821086640.

@tgross35
Copy link
Contributor Author

Regression in ce36a966c79e109dabeef7a47fe68e5294c6d71e

searched nightlies: from nightly-2025-02-05 to nightly-2025-02-18
regressed nightly: nightly-2025-02-18
searched commit range: rust-lang/rust@5bc6231...ce36a96
regressed commit: rust-lang/rust@ce36a96

bisected with cargo-bisect-rustc v0.6.9

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2025-02-05 --script ./bisect.sh --target aarch64-unknown-linux-gnu --preserve -vv

@tgross35
Copy link
Contributor Author

tgross35 commented Feb 19, 2025

That is the LLVM upgrade. The nightly with LLVM20 isn't on godbolt yet but I can reproduce locally. Source:

#![feature(f128)]

#[unsafe(no_mangle)]
pub fn as_int(a: f128) -> u32 {
    a as u32
}

fn main() {
    dbg!(as_int(std::hint::black_box(0.0)));
}

Running (my default nightly is 2025-02-17):

$ rustc main.rs -C linker=aarch64-linux-gnu-gcc --target aarch64-unknown-linux-gnu
$ QEMU_LD_PREFIX=/usr/aarch64-linux-gnu qemu-aarch64 main
[main.rs:14:5] as_int(std::hint::black_box(0.0)) = 4294967295
$ rustc +nightly-2025-02-10 main.rs -C linker=aarch64-linux-gnu-gcc --target aarch64-unknown-linux-gnu
$ QEMU_LD_PREFIX=/usr/aarch64-linux-gnu qemu-aarch64 main
[main.rs:14:5] as_int(std::hint::black_box(0.0)) = 0

@tgross35
Copy link
Contributor Author

Both versions emit the same LLVM IR for the function:

define dso_local i32 @as_int(fp128 %a) unnamed_addr #0 !dbg !721 {
start:
  %a.dbg.spill = alloca [16 x i8], align 16
  store fp128 %a, ptr %a.dbg.spill, align 16
    #dbg_declare(ptr %a.dbg.spill, !727, !DIExpression(), !728)
  %_0 = call i32 @llvm.fptoui.sat.i32.f128(fp128 %a), !dbg !729
  ret i32 %_0, !dbg !730
}

But the result is different. Version with the latest nightly that is unsound:

as_int:
        .cfi_startproc
        sub sp, sp, #96
        str x30, [sp, #80]
        .cfi_def_cfa_offset 96
        .cfi_offset w30, -16
        str q0, [sp]
        str q0, [sp, #64]
        adrp x8, .LCPI15_1
        ldr q1, [x8, :lo12:.LCPI15_1]
        str q1, [sp, #16]
        bl __getf2
        ldr q0, [sp]
        ldr q2, [sp, #16]
        fmov d1, d0
        mov d0, v0.d[1]
        fmov d3, d2
        mov d2, v2.d[1]
        subs w8, w0, #0
        fcsel d1, d1, d3, lt
        str d1, [sp, #48]
        subs w8, w0, #0
        fcsel d0, d0, d2, lt
        str d0, [sp, #56]
        fmov x9, d1
        fmov x8, d0
        mov v0.d[0], x9
        mov v0.d[1], x8
        adrp x8, .LCPI15_0
        ldr q1, [x8, :lo12:.LCPI15_0]
        str q1, [sp, #32]
        bl __gttf2
        ldr q2, [sp, #32]
        ldr d1, [sp, #48]
        ldr d0, [sp, #56]
        fmov d3, d2
        mov d2, v2.d[1]
        subs w8, w0, #0
        fcsel d1, d1, d3, gt
        subs w8, w0, #0
        fcsel d0, d0, d2, gt
        fmov x9, d1
        fmov x8, d0
        mov v0.d[0], x9
        mov v0.d[1], x8
        ldr x30, [sp, #80]
        add sp, sp, #96
        b __fixunstfsi

And with 2025-02-10:

as_int:
        .cfi_startproc
        sub sp, sp, #80
        str x30, [sp, #64]
        .cfi_def_cfa_offset 80
        .cfi_offset w30, -16
        str q0, [sp, #16]
        str q0, [sp, #48]
        adrp x8, .LCPI15_0
        ldr q1, [x8, :lo12:.LCPI15_0]
        bl __getf2
        ldr q0, [sp, #16]
        str w0, [sp, #12]
        bl __fixunstfsi
        ldr w8, [sp, #12]
        ldr q0, [sp, #16]
        subs w8, w8, #0
        mov w8, wzr
        csel w8, w8, w0, lt
        str w8, [sp, #44]
        adrp x8, .LCPI15_1
        ldr q1, [x8, :lo12:.LCPI15_1]
        bl __gttf2
        ldr w8, [sp, #44]
        subs w9, w0, #0
        csinv w0, w8, wzr, le
        ldr x30, [sp, #64]
        add sp, sp, #80
        ret

I can't reproduce this on godbolt however, both llc 19.1 and current trunk (uncertain what version that is) appear to produce the correct output from before our LLVM20 bump https://llvm.godbolt.org/z/eGf7P3bE8. @nikic or @beetrees do you know of anything different among the versions that would cause these codegen differences?

@tgross35
Copy link
Contributor Author

llvm/llvm-project#112370 and llvm/llvm-project#96297 touched that area of code recently and seem like possibilities.

@nikic
Copy link
Contributor

nikic commented Feb 19, 2025

This is a GlobalISel problem, so need to add -O0: https://llvm.godbolt.org/z/PKzdP4876

@nikic
Copy link
Contributor

nikic commented Feb 19, 2025

Filed llvm/llvm-project#127804 for now.

@tgross35
Copy link
Contributor Author

Thanks for looking into it!

Pin aarch64-unknown-linux-gnu to nightly-2025-02-07 until [1] is
resolved.

[1]: llvm/llvm-project#127804
@tgross35
Copy link
Contributor Author

Worked around that issue for now by pinning the nightly.

@tgross35 tgross35 merged commit b2bcfc8 into rust-lang:master Feb 19, 2025
26 checks passed
@tgross35 tgross35 deleted the ci-updates branch February 19, 2025 20:38
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.

2 participants