Page MenuHomePhabricator

[WebAssembly] Fix NaN handling when converting FP types
AbandonedPublic

Authored by luismarques on Nov 9 2020, 12:39 AM.

Details

Summary

Use APFloat's platform-independent FP conversions, instead of C++'s native floating-point type conversions. This addresses an issue that was noted in comments as TODO.
This patch allows the test llvm/test/CodeGen/WebAssembly/immediates.ll to pass on various platforms where it would fail, like RISC-V and MIPS. On RISC-V (hardfloat), the float <-> double conversion canonicalizes the NaN, so some NaN tests in that file would fail.

Diff Detail

Unit TestsFailed

TimeTest
430 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp

Event Timeline

luismarques created this revision.Nov 9 2020, 12:39 AM
luismarques requested review of this revision.Nov 9 2020, 12:39 AM
luismarques retitled this revision from [WebAssembly] Fix NaN handing when converting FP types to [WebAssembly] Fix NaN handling when converting FP types.Nov 9 2020, 2:06 AM
lenary accepted this revision.Jan 14 2021, 9:45 AM

The reasoning behind this seems solid, but I'd like someone from the webassembly backend to chime in.

This revision is now accepted and ready to land.Jan 14 2021, 9:45 AM

The reasoning behind this seems solid, but I'd like someone from the webassembly backend to chime in.

Ping (wasm people).

luismarques abandoned this revision.Mar 20 2021, 4:07 AM

Superseded by D77384 and D97490.
I can confirm that the mips* XFAIL now passes (since D77384 landed on Feb 26). If wasm contributors are still interested in reviewing this patch I can update it to include only the immediate.ll test update.