Skip to content
This repository was archived by the owner on Apr 28, 2025. It is now read-only.

Additional intrinsic optimizations? #214

Closed
Lokathor opened this issue Aug 2, 2019 · 6 comments
Closed

Additional intrinsic optimizations? #214

Lokathor opened this issue Aug 2, 2019 · 6 comments

Comments

@Lokathor
Copy link
Contributor

Lokathor commented Aug 2, 2019

At the moment there's the llvm_intrinsically_optimized! macro which, when using the unstable flag, will call an unstable LLVM intrinsic.

However, there's some opportunities for using intrinsics (edit: hardware intrinsics) in stable, and even in core, if we wanted to reach for SSE / SSE2 / etc when available (compile time detected).

For example, libm defines sqrt with a full software implementation, but if people call it in std they get either (in debug) the sqrtss instruction with some indirection in between or (in release) the sqrtss instruction without any indirection. Based on this, I think it would be fine to have libm also just use the sqrtss instruction when available.

Of course this should probably be behind its own feature flag, but I think it would be a reasonable progression to develop in this direction of using stable hardware intrinsics when possible.

@Lokathor
Copy link
Contributor Author

Lokathor commented Aug 2, 2019

Note: this is related to, but not quite the same as, the #145 discussion

@alexcrichton
Copy link
Member

I think it's fine to improve on the implementations here at any time basically. These intrinsics may not be used in libstd at this time on all platforms, but having optimized implementations would certainly help, and they'll all be well tested anyway

@tgross35
Copy link
Contributor

tgross35 commented Jan 7, 2025

I think it would be better to avoid calling intrinsics or the core equivalent within their own routines - i.e. we shouldn't call intrinsics::fabs from within fabs because that makes it easy to wind up with recursive libcalls if LLVM changes its lowering, or any flags change the lowering, or we get a config wrong.

We can add arch-specific asm or core::arch routines without a problem since we know how those will lower. And then it is fine to call the intrinsics from routines other than their possible libcall lowering - in fact, we should be using these rather than invoking the functions directly, so other routines can benefit from the asm lowering.

The better long term solution to expose this to users is probably, like everything, to move the relevant functions from std to core so the asm lowering is available on all platforms. This should be feasible in the near future for everything that we currently expose to wasm https://github.com/rust-lang/libm/blob/44770b96920557baf38990d2ee4142e166be579d/src/math/arch/wasm32.rs, abs is already in 1.83 (current beta).

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Jan 11, 2025

This should be feasible in the near future for everything that we currently expose to wasm https://github.com/rust-lang/libm/blob/44770b96920557baf38990d2ee4142e166be579d/src/math/arch/wasm32.rs, abs is already in 1.83 (current beta).

Note that nightly Rust has proper core::arch::wasm32 intrinsics for these now (rust-lang/rust#133908) so you could already replace those uses of core::intrinsics today. (Of course, stdarch just uses core::intrinsics to implement those, but it's probably better to de-duplicate something so subtle.)

@tgross35
Copy link
Contributor

Note that nightly Rust has proper core::arch::wasm32 intrinsics for these now (rust-lang/rust#133908) so you could already replace those uses of core::intrinsics today. (Of course, stdarch just uses core::intrinsics to implement those, but it's probably better to de-duplicate something so subtle.)

That's awesome, thanks for pointing it out. Done in #418.

I really wish we had (more stable) wasm asm! to ensure we are avoiding the intrinsic at all, but I'm don't really think anyone knows what the status of this is. I was playing around with it in https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/Wasm.20inline.20assembly.20.20references but couldn't get it to omit an extra local.get (thinking now, maybe I should try naked_asm!).

@tgross35
Copy link
Contributor

tgross35 commented Jan 11, 2025

In any case, w.r.t the original topic: I think we are better off completely avoiding core::intrinsics when implementing the routines they may lower to, but can use them fine in other routines. Using assembly or core::arch is also fine since we know how that lowers. Probably nothing actionable right now (it is a bit of the longer term plan anyway) so I'll close this.

greenhat added a commit to 0xMiden/compiler that referenced this issue Jan 14, 2025
workaround for our old rustc version (fails in fib tests).
See rust-lang/libm#214 (comment)
Besides, that I think we should stick to `wasm32-wasi` everywhere.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants