Repository navigation
Redundant checks in floating point to integer casts on WASM #73591
Description
If you compile the following Rust code to WASM:
pub unsafe fn cast(x: f64) -> u8 {
x.to_int_unchecked()
}it compiles to the following with Rust 1.45:
example::cast:
block
local.get 0
f64.const 0x1p32
f64.lt
local.get 0
f64.const 0x0p0
f64.ge
i32.and
i32.eqz
br_if 0
local.get 0
i32.trunc_f64_u
return
end_block
i32.const 0
end_functionThe same happens in Rust <1.45 if you use as _. In both cases the following LLVM IR is emitted:
define hidden i8 @_ZN7example4cast17he66904755097d668E(double %x) unnamed_addr #0 !dbg !5 {
%0 = fptoui double %x to i8, !dbg !9
ret i8 %0, !dbg !22
}
fptoui is explicitly defined to be UB if an out of range value is provided.
If the value cannot fit in ty2, the result is a poison value.
However it seems like the WASM backend in LLVM ignores that and emits additional bounds checks.
Additionally this gets even worse in Rust 1.45 if you use as _ instead:
example::cast:
local.get 0
f64.const 0x1.fep7
f64.gt
local.set 1
block
block
local.get 0
f64.const 0x0p0
local.get 0
f64.const 0x0p0
f64.gt
f64.select
local.tee 0
f64.const 0x1p32
f64.lt
local.get 0
f64.const 0x0p0
f64.ge
i32.and
i32.eqz
br_if 0
local.get 0
i32.trunc_f64_u
local.set 2
br 1
end_block
i32.const 0
local.set 2
end_block
i32.const -1
local.get 2
local.get 1
i32.select
end_functionThis means that in the end in the WASM VM will do 3 range checks (the one emitted by rust, the one emitted by the llvm backend, and the one it needs to do to possibly trap).
Additionally Rust's saturating checks don't play well with WASM's nontrapping-fptoint target-feature as there's still redundant checks:
example::cast:
i64.const 9223372036854775807
local.get 0
f32.const -0x1p63
local.get 0
f32.const -0x1p63
f32.gt
f32.select
i64.trunc_sat_f32_s
local.get 0
f32.const 0x1.fffffep62
f32.gt
i64.select
i64.const 0
local.get 0
local.get 0
f32.eq
i64.select
end_function
Activity
CryZe commentedon Jun 22, 2020
It seems to come down to this commit by @sunfishcode in LLVM llvm/llvm-project@cdd48b8
I fail to understand the justification here:
If the LLVM opcode doesn't define the behavior for invalid results, then why is the WASM backend trying to paper over the UB and instead introduces checks to supposedly trap? (which I don't think it even does, it just returns dummy values)
sunfishcode commentedon Jun 22, 2020
In the context of that comment, "undefined result" doesn't mean "undefined behavior". It means the operation produces a return value, without trapping, but the specific value is undefined. Since's the wasm operator in question traps in some questions, extras checks are required to prevent trapping, to emulate the LLVM IR semantics.
#71269 has now landed, so Rust's semantics for fp-to-int conversion match those of wasm's nontrapping-fptoint feature, which is now part of wasm's core spec, though not all engines implement it yet. So for anyone interested in working on this, the path forward is to teach the Rust backend to take advantage of this wasm feature, when present, to avoid emitting the extra range checks.
CryZe commentedon Jun 22, 2020
So from what I'm understanding from more code, LLVM could be reordering fptoui / fptosi to be speculatively executed earlier, which would mean the code traps when it possibly shouldn't have, as that cast may have never been executed. Is that correct?
There also seem to be LLVM intrinsics in the WASM backend now such as
i32 @llvm.wasm.trunc.signed.i32.f32(float). Can those be speculatively executed? If no, the code should probably be lowered like this:-nontrapping-fptoint+nontrapping-fptoint.to_int_unchecked()llvm.wasm.trunc.*llvm.wasm.trunc.*as _llvm.wasm.trunc.*+ manual saturation checksllvm.wasm.trunc.saturate.*If yes, it should probably be this:
-nontrapping-fptoint+nontrapping-fptoint.to_int_unchecked()fptosi/fptouillvm.wasm.trunc.saturate.*as _llvm.wasm.trunc.*+ manual saturation checks(or maybe that intrinsic is still dangerous in that case?)
llvm.wasm.trunc.saturate.*sunfishcode commentedon Jun 22, 2020
Yes, LLVM can reorder and speculate fptosi/fptoui as it assumes they don't trap.
@llvm.wasm.trunc.signed.i32.f32can trap, so it cannot be speculatively executed in general.Rollup merge of rust-lang#73724 - CryZe:wasm-saturating-casts, r=alex…
Rollup merge of rust-lang#73724 - CryZe:wasm-saturating-casts, r=alex…
Rollup merge of rust-lang#73724 - CryZe:wasm-saturating-casts, r=alex…
Rollup merge of rust-lang#73724 - CryZe:wasm-saturating-casts, r=alex…
Improve codegen for unchecked float casts on wasm
alexcrichton commentedon Jul 22, 2020
I've posted #74659 to improve the unchecked
-nontrapping-fptointcaseRollup merge of rust-lang#74659 - alexcrichton:wasm-codegen, r=varkor
alexcrichton commentedon Jul 23, 2020
Ok, I've improved the last case at #74695 and I think that's good enough to close this out.
Auto merge of rust-lang#74695 - alexcrichton:more-wasm-float-cast-fix…