Configure f16 and f128 support for WebAssembly

[kleisauke]

See: #652 (comment)

Tested with:

Details

$ curl https://sh.rustup.rs -sSf | sh -s -- -y --profile minimal --target wasm32-unknown-emscripten --default-toolchain nightly-2024-08-11 --component rust-src
$ source "$HOME/.cargo/env"
$ cargo new foo
$ cd foo
$ RUSTFLAGS="-Clink-arg=-Wl,--fatal-warnings" cargo build --release -Zbuild-std --target wasm32-unknown-emscripten
[...]
wasm-ld: error: function signature mismatch: __truncsfhf2
>>> defined as (f32) -> i32 in /home/kleisauke/foo/target/wasm32-unknown-emscripten/release/deps/libcompiler_builtins-903959ee8c3bdcea.rlib(compiler_builtins-903959ee8c3bdcea.compiler_builtins.f8125c6c5a7ecb3b-cgu.08.rcgu.o)
>>> defined as (f32) -> f32 in /home/kleisauke/foo/target/wasm32-unknown-emscripten/release/deps/libcompiler_builtins-903959ee8c3bdcea.rlib(compiler_builtins-903959ee8c3bdcea.compiler_builtins.f8125c6c5a7ecb3b-cgu.04.rcgu.o)

wasm-ld: error: function signature mismatch: __extendhfsf2
>>> defined as (i32) -> f32 in /home/kleisauke/foo/target/wasm32-unknown-emscripten/release/deps/libcompiler_builtins-903959ee8c3bdcea.rlib(compiler_builtins-903959ee8c3bdcea.compiler_builtins.f8125c6c5a7ecb3b-cgu.03.rcgu.o)
>>> defined as (f32) -> f32 in /home/kleisauke/foo/target/wasm32-unknown-emscripten/release/deps/libcompiler_builtins-903959ee8c3bdcea.rlib(compiler_builtins-903959ee8c3bdcea.compiler_builtins.f8125c6c5a7ecb3b-cgu.08.rcgu.o)
[...]
error: could not compile `foo` (bin "foo") due to 1 previous error
$ cat >> $(rustc --print sysroot)/lib/rustlib/src/rust/library/Cargo.toml <<EOL
compiler_builtins = { git = "https://github.com/kleisauke/compiler-builtins.git", branch = "wasm-f16-f128-feature" }
EOL
$ RUSTFLAGS="-Clink-arg=-Wl,--fatal-warnings" cargo build --release -Zbuild-std --target wasm32-unknown-emscripten
[...]
    Finished `release` profile [optimized] target(s) in 7.85s

[kleisauke]

I've updated the reproducer in the initial post. It seems that the function signature mismatch issues are treated as warnings by default, but they become visible when passing RUSTFLAGS="-Clink-arg=-Wl,--fatal-warnings". This behavior is automatically enforced when using Emscripten's STRICT mode.
https://github.com/emscripten-core/emscripten/blob/c614fa57ea8048a34341891dbd6f99fb78b6869a/tools/building.py#L267-L268

CI is still failing for wasm32-unknown-unknown. I think we need to conditionally disable this test somehow:

compiler-builtins/testcrate/src/bench.rs

Lines 353 to 354 in 3ad4d9c

	#[cfg(not(feature = "no-f16-f128"))]
	impl_testio!(float f16, f128);

[tgross35]

I've updated the reproducer in the initial post. It seems that the function signature mismatch issues are treated as warnings by default, but they become visible when passing RUSTFLAGS="-Clink-arg=-Wl,--fatal-warnings". This behavior is automatically enforced when using Emscripten's STRICT mode. https://github.com/emscripten-core/emscripten/blob/c614fa57ea8048a34341891dbd6f99fb78b6869a/tools/building.py#L267-L268

Just to see, I tested making linker errors fatal on all targets in #667. wasm32-unknown-unknown seems to fail for other reasons.

CI is still failing for wasm32-unknown-unknown. I think we need to conditionally disable this test somehow:

compiler-builtins/testcrate/src/bench.rs

Lines 353 to 354 in 3ad4d9c

	#[cfg(not(feature = "no-f16-f128"))]
	impl_testio!(float f16, f128);

Ah, these will be the only targets that are disabled in /build.rs but tested in CI. I think we need to emit the same enable_f16 and enable_f128 feature in testcrate, then gate relevant things. To avoid duplicating the logic you should be able to do something like:

// testcrate/build.rs

mod builtins_build {
    include!("../build.rs");
}

fn main() {
    // ...

    // will need to mark some things public in `/build.rs`
    builtins_build::configure_f16_f128(builtins_build::Target::from_env());
}

I would only do the enable_f16 gating for now since enable_f128 isn't relevant for testcrate at this time.

[kleisauke]

Thanks! I just pushed commit 42ac64e. However, CI still fails for the wasm32-unknown-unknown target, curiously.

[tgross35]

Thanks! I just pushed commit 42ac64e. However, CI still fails for the wasm32-unknown-unknown target, curiously.

Unfortunately the files in benches/ also need to be updated, so anything that tests f16 should get gated behind f16_enabled - both the float_bench! and the criterion_group! invocations will need adjusting. The testcrate/src/bench.rs change is needed too.

Some of the benches already have different criterion_group! per cfg, e.g.

compiler-builtins/testcrate/benches/float_extend.rs

Lines 85 to 102 in 3ad4d9c

	#[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))]
	criterion_group!(
	float_extend,
	extend_f16_f32,
	extend_f16_f128,
	extend_f32_f64,
	extend_f32_f128,
	extend_f64_f128,
	);
	// FIXME(#655): `f16` tests disabled until we can bootstrap symbols
	#[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))]
	criterion_group!(
	float_extend,
	extend_f32_f64,
	extend_f32_f128,
	extend_f64_f128,
	);

. When updating, it is probably better to not use criterion_group and just configure manually, something like

pub fn float_extend() {
    let mut criterion = Criterion::default().configure_from_args();

    #[cfg(f16_enabled)]
    {
        ...
    }
    
    extend_f32_f64(&mut criterion);
    extend_f32_f64(&mut criterion);
}

[kleisauke]

All done! This is now ready for review. Thanks to @beetrees and @tgross35 for the help. :)

[tgross35]

Thanks! You can submit a PR to update rust-lang/rust to 0.1.119 once #668 merges.

[tgross35]

Ah, publish failed because build.rs uses multiple files. @kleisauke can you submit a PR to either add the new file to the include list

compiler-builtins/Cargo.toml

Line 17 in a4cd445

'/build.rs',

, or have testcrate/build.rs just include the entire /build.rs as mentioned here #665 (comment)?

[kleisauke]

Apologies for the breakage. I'll submit a PR to add the new file to the include list.

Initially, I attempted to include the entire /build.rs into testcrate/build.rs, but that approach failed, as shown here:
https://github.com/kleisauke/compiler-builtins/actions/runs/10356628453/job/28666867698#step:13:519

This is why I decided to move that logic into a separate file.

[tgross35]

No worries, thanks for the clarification. I'll take a look when it is up.

[kleisauke]

I just opened PR #669 for this.

[tgross35]

0.1.119 published with that fix, feel free to submit a rust-lang/rust PR.