Page MenuHomePhabricator

[WebAssembly] Use standard intrinsics for f32x4 and f64x2 ops
ClosedPublic

Authored by tlively on Tue, Apr 13, 1:55 PM.

Details

Summary

Now that these instructions are no longer prototypes, we do not need to be
careful about keeping them opt-in and can use the standard LLVM infrastructure
for them. This commit removes the bespoke intrinsics we were using to represent
these operations in favor of the corresponding target-independent intrinsics.
The clang builtins are preserved because there is no standard way to easily
represent these operations in C/C++.

For consistency with the scalar codegen in the Wasm backend, the intrinsic used
to represent {f32x4,f64x2}.nearest is @llvm.nearbyint even though
@llvm.roundeven better captures the semantics of the underlying Wasm
instruction. Replacing our use of @llvm.nearbyint with use of @llvm.roundeven is
left to a potential future patch.

Diff Detail

Event Timeline

tlively created this revision.Tue, Apr 13, 1:55 PM
tlively requested review of this revision.Tue, Apr 13, 1:55 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptTue, Apr 13, 1:55 PM
srj added a subscriber: srj.Tue, Apr 13, 4:05 PM
sunfish accepted this revision.Tue, Apr 13, 4:15 PM

This looks good to me! Could you briefly comment here on what the issue with llvm.roundeven is?

This revision is now accepted and ready to land.Tue, Apr 13, 4:15 PM

The Wasm rounding semantics (https://webassembly.github.io/spec/core/exec/numerics.html#op-fnearest) are essentially the same as the semantics for roundeven (https://llvm.org/docs/LangRef.html#llvm-roundeven-intrinsic, added to LLVM in May 2020), so that would be the logical ISD opcode to use. Nearbyint is less appropriate because its behavior is supposed to depend on the current rounding mode. Arguably this is fine because Wasm only has the one rounding mode, but roundeven is more unambiguously correct. However, the ISel for f32.nearest and f64.nearest predates roundeven being available in LLVM and uses nearbyint instead, so I'm just matching that here. If folks think it's a good idea, I will make a follow-up PR that switches to using roundeven for both scalar and SIMD ISel.

Great, thanks! And yes, switching to roundeven for both scalar and SIMD ISel sounds right to me.

aheejin accepted this revision.Tue, Apr 13, 7:35 PM