intrinsics: Add a fallback for non-const libm float functions#150946
intrinsics: Add a fallback for non-const libm float functions#150946tgross35 wants to merge 1 commit intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-review
| #[rustc_intrinsic_const_stable_indirect] | ||
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub const fn fmaf128(a: f128, b: f128, c: f128) -> f128; |
There was a problem hiding this comment.
Is there a way to provide a non-const fallback for const intrinsics? In cases where CTFE definitely has to hook it rather than using the fallback, like here.
Not the worst thing if not.
There was a problem hiding this comment.
I don't think there is -- please file an issue.
This comment has been minimized.
This comment has been minimized.
A number of float operations from libm have intrinsics for optimization, but it is also okay to just call the libm functions directly. Add a fallback for these cases, including converting to/from another float size where needed, so the backends don't need to override these.
tgross35
force-pushed
the
float-intrinsic-fallback
branch
from
d533ef5 to
90b9d6a
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Huh - so |
|
At a glance, your reasoning is completely backwards. The fallback body for |
|
It shouldn't be; the |
|
The changes LGTM. However I can't actually check whether what you say about what is available where is correct -- you would be the person I ask about things like that. ;) |
|
https://github.com/rust-lang/compiler-builtins/blob/65624df7f55db9b7b494fbe3aa9dcea0a743eea4/compiler-builtins/src/math/mod.rs is what controls what we sometimes/always provide, so the module root symbols and |
Yes, that's the problem. The body for the function is not available in the crate compiler-builtins when compiling compiler-builtins. Remember the the rule is to ban linkage against other crates and the point of these extern declarations is to use linkage. |
|
☔ The latest upstream changes (presumably #151490) made this pull request unmergeable. Please resolve the merge conflicts. |
Forgot to follow up here. In this case it there isn't anything technically incorrect right, and instead it's a limitation of the knowledge the compiler has to emit that error? Because c-b itself provides the It probably goes without saying but libm isn't calling the |
|
Oh I see, thanks for clarifying. I think this is actually a limitation of the check logic itself. The problem is that you have an item defined in an upstream crate, which is not So the missing check is whether the |
|
I threw this together to convince myself that this can work, and it seems to work. diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs
index c8aa7c04585..3fc0411d2be 100644
--- a/compiler/rustc_codegen_ssa/src/base.rs
+++ b/compiler/rustc_codegen_ssa/src/base.rs
@@ -867,6 +867,10 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
/// unlinkable calls.
///
/// Note that calls to LLVM intrinsics are uniquely okay because they won't make it to the linker.
+/// Note also that calls to foreign items that are actually exported by the local crate are also
+/// okay. This situation arises because compiler-builtins calls functions in core that are #[inline]
+/// wrappers for extern "C" declarations in core, which resolve to a symbol exported by
+/// compiler-builtins.
pub fn is_call_from_compiler_builtins_to_upstream_monomorphization<'tcx>(
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
@@ -879,11 +883,19 @@ fn is_llvm_intrinsic(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
}
}
+ fn is_extern_call_to_local_crate<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> bool {
+ tcx.is_foreign_item(instance.def_id())
+ && tcx.exported_non_generic_symbols(LOCAL_CRATE).iter().any(|(sym, _info)| {
+ sym.symbol_name_for_local_instance(tcx) == tcx.symbol_name(instance)
+ })
+ }
+
let def_id = instance.def_id();
!def_id.is_local()
&& tcx.is_compiler_builtins(LOCAL_CRATE)
&& !is_llvm_intrinsic(tcx, def_id)
&& !tcx.should_codegen_locally(instance)
+ && !is_extern_call_to_local_crate(tcx, instance)
}
impl CrateInfo {The linear search of all exported non-generic symbols seems bad. It's only done a handful of times so it doesn't have a visible impact on compile time of compiler-builtins... yet. |
5 participants
A number of float operations from libm have intrinsics for optimization, but it is also okay to just call the libm functions directly. Add a fallback for these cases, including converting to/from another float size where needed, so the backends don't need to override these.
r? @RalfJung