Skip to content

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

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

Closed
wants to merge 2 commits into 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.

This is going to be needed to fix rust-lang/compiler-builtins#649.

I was able to test this locally by changing some other code (basically having an implementation of _mm_sqrt_pd that calls the local sqrt function on all elements of the vector).
Do you have any ideas for how to add a test for this? Perhaps by including this implementation of _mm_sqrt_pd behind a #[cfg]?

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 force-pushed the fix/infinite-recursion-sqrt branch from 9f5e58d to ab5f6e3 Compare April 5, 2025 15:33
@antoyo antoyo marked this pull request as ready for review April 5, 2025 18:32
@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

I think this PR shouldn't be needed; usage of force-soft-floats in source was replaced with arch_enabled to simplify the logic, which select_implementation! already guards against

// By default, never use arch-specific implementations if we have force-soft-floats
#[cfg(arch_enabled)]
. The config is set here

libm/configure.rs

Lines 79 to 88 in c9672e5

/// Simplify the feature logic for enabling arch-specific features so code only needs to use
/// `cfg(arch_enabled)`.
fn emit_arch_cfg() {
println!("cargo:rustc-check-cfg=cfg(arch_enabled)");
// Enabled by default via the "arch" feature, `force-soft-floats` overrides to disable.
if cfg!(feature = "arch") && !cfg!(feature = "force-soft-floats") {
println!("cargo:rustc-cfg=arch_enabled");
}
}
, compiler-builtins probably just needs to be fixed to have the same logic.

@antoyo
Copy link
Author

antoyo commented Apr 9, 2025

I think this PR shouldn't be needed; usage of force-soft-floats in source was replaced with arch_enabled to simplify the logic, which select_implementation! already guards against

I thought arch_enabled would enable the usage of assembly. So won't this disable assembly in addition to hardware floats?

@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

(replying at rust-lang/compiler-builtins#810 since I had a better answer there)

@antoyo
Copy link
Author

antoyo commented Apr 18, 2025

Closing since this was fixed elsewhere.

@antoyo antoyo closed this Apr 18, 2025
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