Skip to content

Reenable the use of the force-soft-floats feature #810

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

antoyo
Copy link

@antoyo antoyo commented Apr 5, 2025

This fixes an infinite recursion in sqrt for codegen backends that implement intrinsics like simd_fsqrt by calling sqrt on every element of the vector.

Fix #649

This requires rust-lang/libm#532 to fix the issue, so I'll need to update the submodule when this PR is merged.

This fixes an infinite recursion in sqrt for codegen backends that
implement intrinsics like simd_fsqrt by calling sqrt on every element of
the vector.
@antoyo antoyo marked this pull request as draft April 5, 2025 14:04
@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

Sorry about the regression here. Could this be temporally fixed by setting the compiler-builtins-no-asm feature when building with cg_gcc https://github.com/rust-lang/rust/blob/d4f880f8ce832cd7560bb2f1ebc34f967055ffd7/library/std/Cargo.toml#L99? I think bootstrap has a way to configure this.

The change in libm was made to move away from subtractive features like force-soft-float because that doesn't work well with Cargo and config was getting complicated (now gone in libm but still exists in compiler-builtins). If something like this is needed, I'd prefer splitting out separate features to enable assembly and to enable the rest of core::arch, rather than adding back force-soft-float. I can do this work, just let me know if no-asm is an option.

@antoyo
Copy link
Author

antoyo commented Apr 9, 2025

just let me know if no-asm is an option.

Just to make sure, this will disable some optimizations, right?
If so, I'd prefer to not use no-asm since rustc_codegen_gcc supports inline assembly.

@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

Yeah you're right, that would affect compiler-builtins. It's not a problem in libm since there are no uses of asm! though.

So what might be best is to comment out println!("cargo:rustc-cfg=arch_enabled"); with a FIXME to add this back in the future once libm drops all core::arch calls. This is the plan after the functions are in core (I'm hoping rust-lang/rust#138087 can go to stable after only a couple of cycles).

The existing mention of force-soft-floats can be deleted too, it isn't used in either crate anymore so I guess I forgot to clean it up.

@antoyo
Copy link
Author

antoyo commented Apr 9, 2025

So what might be best is to comment out println!("cargo:rustc-cfg=arch_enabled"); with a FIXME to add this back in the future once libm drops all core::arch calls.

Ok, do you want me to make a PR for this or will you do it?
I might need more details if you want me to make the PR.

Thanks!

@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

You can probably just update this PR, something like:

    // The arch module may contain assembly.
    if !cfg!(feature = "no-asm") {
        // FIXME: when part of builtins, `libm` can use assembly but cannot use vector intrinsics
        // from `core::arch` in order to avoid recursion problems. Once basic `libm` functions
        // are available in `core`, we will be able to remove these problematic calls and add back
        // `arch_enabled`.
        // Cc: <https://github.com/rust-lang/compiler-builtins/issues/649>
        // println!("cargo:rustc-cfg=arch_enabled");
    }

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.

Infinite recursion in sqrt
2 participants