Skip to content

Conversation

@bluenote10
Copy link
Contributor

This PR improves the Rust codegen for targeting wasm, following up on ideas from #1118.

Basically now the generated code works for both non-wasm and wasm compilation. (My requirement was to have the same generated code work under both.)

To demonstrate, the generated code now looks like this:

#[cfg(not(target_arch = "wasm32"))] // Compile ffi bindings only on non-wasm targets
mod ffi {
    use std::os::raw::c_float;
    // Conditionally compile the link attribute only on non-Windows platforms
    #[cfg_attr(not(target_os = "windows"), link(name = "m"))]
    unsafe extern "C" {
        pub fn remainderf(from: c_float, to: c_float) -> c_float;
        pub fn rintf(val: c_float) -> c_float;
    }
}
fn remainder_f32(from: f32, to: f32) -> f32 {
    #[cfg(not(target_arch = "wasm32"))] // non-wasm targets use ffi bindings
    unsafe { ffi::remainderf(from, to) }
    #[cfg(target_arch = "wasm32")] // wasm relies on libm
    libm::remainderf(from, to)
}
fn rint_f32(val: f32) -> f32 {
    #[cfg(not(target_arch = "wasm32"))] // non-wasm targets use ffi bindings
    unsafe { ffi::rintf(val) }
    #[cfg(target_arch = "wasm32")] // wasm relies on libm
    libm::rintf(val)
}

This follows @mitchmindtree's idea in #1118 (comment) and works for both use cases:

  • native targets: They use the ffi and linking to lm as before.
  • wasm target: They delegate to libm, a pure Rust crate offering the same functionality. Of course, wasm users need to add libm as a dependency.

Note that this makes the -rnlm/--rust-no-libm command line args (and gRustNoLibm internally) more or less obsolete. Removing it would be a breaking change though for someone expecting these command line arguments to exist. I didn't remove it yet, but if the breaking change is considered minor enough, we could do so in a follow-up. This would be more or less the inverse of ad2e156.

@sletz
Copy link
Member

sletz commented Dec 28, 2025

@crop2000 You added -rnlm/--rust-no-libm right ? So can we remove them ?

@bluenote10
Copy link
Contributor Author

@crop2000 You added -rnlm/--rust-no-libm right ?

@sletz If I remember correctly @crop2000 had added the -rnt/--rust-no-faustdsp-trait (which is independent), but I believe -rnlm/--rust-no-libm was added by you in ad2e156 more or less as a quick fix for #1118.

@sletz
Copy link
Member

sletz commented Dec 28, 2025

OK, so better wait for @mitchmindtree feedback then ?

@bluenote10
Copy link
Contributor Author

OK, so better wait for @mitchmindtree feedback then ?

My idea was to merge this approach and release it so that it's easy for people to use it and get rid of their --rust-no-libm usages first. Then we can still make the decision whether to remove it at a later point based on feedback.

@sletz
Copy link
Member

sletz commented Dec 28, 2025

OK done, thanks.

@sletz sletz closed this Dec 28, 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.

2 participants