rust-lang / compiler-builtins

Porting `compiler-rt` intrinsics to Rust
https://github.com/rust-lang/rust/issues/35437
Other
354 stars 203 forks source link

Infinite recursion in sqrt #649

Open antoyo opened 1 month ago

antoyo commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 4 weeks ago

I believe this should be fixed now?

antoyo commented 4 weeks ago

I will test it in a few days and confirm.