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

Infinite recursion in sqrt #649

Open
antoyo opened this issue Jul 29, 2024 · 4 comments
Open

Infinite recursion in sqrt #649

antoyo opened this issue Jul 29, 2024 · 4 comments

Comments

@antoyo
Copy link

antoyo commented Jul 29, 2024

Hi.
This is a continuation of this Zulip discussion as it seems this might be a bug in compiler-builtins (please close if this is not the case).

The context was:

I'm trying to update compiler-builtins in rustc_codegen_gcc that was pinned to 0.1.109 until the support for f16 and f128 was implemented.
I used to not generate symbols like sqrt from compiler_builtins (instead, we were using the one from libgcc) in order to avoid infinite recursion like:

compiler_builtins::math::libm::sqrt::sqrt
compiler_builtins::math::sqrt
sqrt
core::core_arch::x86::sse2::_mm_sqrt_pd

because rustc_codegen_gcc implements SIMD sqrt by calling sqrt on each element of the vector for now.
But with the new update, it seems I cannot do that anymore. This workaround seemed like a hack anyway.

@Amanieu mentioned the following which might be the solution:

Really compiler_builtins shouldn't be calling the native instruction, it should only contain the fallback.
Since the compiler is responsible for emitting the proper instruction and not calling the builtin in the first place.

Thanks.

@tgross35
Copy link
Contributor

Do you know if it a change in builtins or a change in cg_gcc that started this? The builtins implementation comes from here https://github.com/rust-lang/libm/blob/279e5f6abe0a2ca9066962d9ec894f0df1f417ac/src/math/sqrt.rs which hasn't been updated in two years except for some typos. I guess maybe something in the config changed.

@antoyo
Copy link
Author

antoyo commented Jul 29, 2024

You can see my PR that attemps to do the update here.

My understanding is that it's caused by a change in compiler-builtins because at first, I only tried updating its version.
Just trying to update the version will cause the error on other builtins:

libgccjit.so: error: : gcc_jit_function_new_block: cannot add block to an imported function
thread 'rustc' panicked at /home/bouanto/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gccjit-2.1.0/src/function.rs:183:17:
gcc_jit_function_new_block: cannot add block to an imported function (rint)
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cf2df68d1f5e56803c97d91e2b1a9f1c9923c533/library/std/src/panicking.rs:661:5
   1: core::panicking::panic_fmt
             at /rustc/cf2df68d1f5e56803c97d91e2b1a9f1c9923c533/library/core/src/panicking.rs:74:14
   2: gccjit::function::Function::new_block
             at /home/bouanto/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gccjit-2.1.0/src/function.rs:183:17
   3: <rustc_codegen_gcc::builder::Builder as rustc_codegen_ssa::traits::builder::BuilderMethods>::append_block
             at /home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc/src/builder.rs:545:9
   4: rustc_codegen_ssa::mir::codegen_mir
             at /rustc/cf2df68d1f5e56803c97d91e2b1a9f1c9923c533/compiler/rustc_codegen_ssa/src/mir/mod.rs:176:22
   5: rustc_codegen_ssa::base::codegen_instance
             at /rustc/cf2df68d1f5e56803c97d91e2b1a9f1c9923c533/compiler/rustc_codegen_ssa/src/base.rs:388:5
   6: <rustc_middle::mir::mono::MonoItem as rustc_codegen_ssa::mono_item::MonoItemExt>::define
             at /rustc/cf2df68d1f5e56803c97d91e2b1a9f1c9923c533/compiler/rustc_codegen_ssa/src/mono_item.rs:106:17
   7: rustc_codegen_gcc::base::compile_codegen_unit::module_codegen
             at /home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc/src/base.rs:211:17
   8: rustc_query_system::dep_graph::graph::DepGraph<D>::with_task
             at /rustc/cf2df68d1f5e56803c97d91e2b1a9f1c9923c533/compiler/rustc_query_system/src/dep_graph/graph.rs:291:22
   9: rustc_codegen_gcc::base::compile_codegen_unit
             at /home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc/src/base.rs:76:23
  10: <rustc_codegen_gcc::GccCodegenBackend as rustc_codegen_ssa::traits::backend::ExtraBackendMethods>::compile_codegen_unit
             at /home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc/src/lib.rs:322:9
  11: rustc_codegen_ssa::base::codegen_crate
             at /rustc/cf2df68d1f5e56803c97d91e2b1a9f1c9923c533/compiler/rustc_codegen_ssa/src/base.rs:748:34
  12: <rustc_codegen_gcc::GccCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::codegen_crate
             at /home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc/src/lib.rs:237:19
  13: <rustc_interface::queries::Queries>::codegen_and_build_linker
  14: rustc_interface::interface::run_compiler::<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

and I got to this issue with sqrt by attempting to remove my workaround.

Since this workaround is hackish anyway, I'd like to resolve the issue for real, but if you want, I can try to find exactly which version caused the issue.

I've seen some commits changing the #[cfg] like this one that might be what has caused my problem.

@Amanieu
Copy link
Member

Amanieu commented Aug 10, 2024

I believe this should be fixed now?

@antoyo
Copy link
Author

antoyo commented Aug 10, 2024

I will test it in a few days and confirm.

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

No branches or pull requests

3 participants