WASM float to int performance regression since 1.53.0 #87643

Activity

1. CryZe

   added

   C-bugCategory: This is a bug.Category: This is a bug.

   on Jul 30, 2021
2. CryZe commented on Jul 30, 2021

   

   CryZe

   on Jul 30, 2021

   Last edited by CryZe

   ContributorAuthor

   More actions

   Though looking at the LLVM IR it seems like the difference is that 1.52.0 is our own rustc saturation implementation and since 1.53 it's LLVM 12's saturation implementation, which seems to be worse?
3. alexcrichton commented on Jul 30, 2021

   

   alexcrichton

   on Jul 30, 2021

   Last edited by alexcrichton

   Member

   More actions

   Thanks for the heads up, this is behaving as "expected" although the expectation wasn't really thoroughly evaluated by me.

   As you've discovered the main difference (which you can see in that diff view with --emit llvm-ir) is that with Rust 1.52 and prior (pre-those PRs) rustc manually emitted LLVM IR that did the right thing. Rust 1.53.0 and later we're calling the LLVM intrinsic for saturating float-to-int conversion.

   In that sense codegen is behaving as expected, and I believe at the time I diff'd the two (rustc's old codegen and LLVM's built-in intrinsic codegen) and saw they were different but assume that the difference was negligible. Have you measured the LLVM-intrinsic-generated code to have worse performance? (I see it has a few extra instructions but I'd be curious to put concrete numbers on it if possible)

   If LLVM has worse performance I think it'd be good to open an issue upstream with them and see if we can improve it upstream, but if it's critical and it's too difficult to land upstream then we can perhaps re-land the wasm-specific bits for rustc.

   (it's also be best if Safari implemented the nontrapping-fptoint extensions so we could consider turning that on by default...)
4. CryZe commented on Jul 30, 2021

   

   CryZe

   on Jul 30, 2021

   Last edited by CryZe

   ContributorAuthor

   More actions

   I didn't do any benchmarks, but it seems like it suffers from the same problem as our original codegen. This is roughly the new WASM translated into Rusty pseudo code:

   // General ifs to do the saturation
   if x.is_nan() {
       0
   } else if x >= 0x1.fffffep30 {
       2147483647
   } else if x >= -0x1p31 {
       // Protection against trapping
       if x.abs() < 0x1p31 {
           (int)x
       } else {
           -2147483648
       }
   } else {
       -2147483648
   }

   This protection against trapping shouldn't be there, the saturation code already checked for all the edge cases (though technically a lot of these are selects, which they can't be if you remove the protection code, which may have some performance implications?). This is probably because this lowering of the saturation casts is backend independent and yet again the WASM backend doesn't know anything about it, so it still protects itself from the trapping. So yeah this definitely can be improved. I'll look into raising an upstream issue I guess.

   It's not critical at all, it's just something that we stumbled upon in some Twitter discussion.
5. adrian17 commented on Jul 31, 2021

   

   adrian17

   on Jul 31, 2021

   Last edited by adrian17

   More actions

   Both Rust and WASM introduced saturating float to int casts

   Don't you need to opt-in on the wasm side with -C target-feature=+nontrapping-fptoint ? After adding this to your godbolt link, on nightly (but not 1.52), it'll compile down to:

   example::cast:
           local.get       0
           i32.trunc_sat_f32_s
           end_function
   

   (of course it doesn't change that without this feature, there's a regression)
6. CryZe commented on Jul 31, 2021

   

   CryZe

   on Jul 31, 2021

   ContributorAuthor

   More actions

   Yeah this issue is only concerned with not having the WASM feature active. It should still produce reasonably good code then. Atm it's not as good as before 1.53.
7. sanxiyn

   added

   O-wasmTarget: WASM (WebAssembly), http://webassembly.org/Target: WASM (WebAssembly), http://webassembly.org/

   on Dec 17, 2021