[WebAssembly] Fix NaN handling when converting FP types

[luismarques]

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.

[luismarques]

Ping.

[lenary]

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

[luismarques]

In D91052#2498657, @lenary wrote:

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

Ping (wasm people).

[luismarques]

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.

llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp

if (Op.isReg()) {
     if (OpNo < MII.get(MI->getOpcode()).getNumDefs() || IsVariadicDef)
       O << '=';
   } else if (Op.isImm()) {
     O << Op.getImm();
   } else if (Op.isFPImm()) {
     const MCInstrDesc &Desc = MII.get(MI->getOpcode());
     const MCOperandInfo &Info = Desc.OpInfo[OpNo];
     if (Info.OperandType == WebAssembly::OPERAND_F32IMM) {
-      // TODO: MC converts all floating point immediate operands to double.
-      // This is fine for numeric values, but may cause NaNs to change bits.
-      O << ::toString(APFloat(float(Op.getFPImm())));
+      // MC floating point immediate operands are always encoded with the type
+      // double. To avoid changes to NaN values that can occur when converting
+      // a C++ double to a float, we convert the immediate to a float using
+      // APFloat's architecture-independent FP value manipulation code.
+      APFloat APFloatImm = APFloat(Op.getFPImm());
+      bool Ignored;
+      APFloatImm.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven,
+                         &Ignored);
+      O << ::toString(APFloatImm);
     } else {
       assert(Info.OperandType == WebAssembly::OPERAND_F64IMM);
       O << ::toString(APFloat(Op.getFPImm()));
     }
   } else {
     assert(Op.isExpr() && "unknown operand kind in printOperand");
     // call_indirect instructions have a TYPEINDEX operand that we print
     // as a signature here, such that the assembler can recover this

llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp

if (MO.isReg()) {
         }
       } else {
         encodeULEB128(uint64_t(MO.getImm()), OS);
       }
 
     } else if (MO.isFPImm()) {
       const MCOperandInfo &Info = Desc.OpInfo[I];
       if (Info.OperandType == WebAssembly::OPERAND_F32IMM) {
-        // TODO: MC converts all floating point immediate operands to double.
-        // This is fine for numeric values, but may cause NaNs to change bits.
-        auto F = float(MO.getFPImm());
+        // MC floating point immediate operands are always encoded with the type
+        // double. To avoid changes to NaN values that can occur when converting
+        // a C++ double to a float, we convert the immediate to a float using
+        // APFloat's architecture-independent FP value manipulation code.
+        APFloat APFloatImm = APFloat(MO.getFPImm());
+        bool Ignored;
+        APFloatImm.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven,
+                           &Ignored);
+        float F = APFloatImm.convertToFloat();
         support::endian::write<float>(OS, F, support::little);
       } else {
         assert(Info.OperandType == WebAssembly::OPERAND_F64IMM);
         double D = MO.getFPImm();
         support::endian::write<double>(OS, D, support::little);
       }
 
     } else if (MO.isExpr()) {

llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

case MachineOperand::MO_Immediate: {
             break;
           }
         }
       }
       MCOp = MCOperand::createImm(MO.getImm());
       break;
     }
     case MachineOperand::MO_FPImmediate: {
-      // TODO: MC converts all floating point immediate operands to double.
-      // This is fine for numeric values, but may cause NaNs to change bits.
       const ConstantFP *Imm = MO.getFPImm();
-      if (Imm->getType()->isFloatTy())
-        MCOp = MCOperand::createFPImm(Imm->getValueAPF().convertToFloat());
-      else if (Imm->getType()->isDoubleTy())
-        MCOp = MCOperand::createFPImm(Imm->getValueAPF().convertToDouble());
-      else
-        llvm_unreachable("unknown floating point immediate type");
+      APFloat APFloatImm = Imm->getValueAPF();
+      if (Imm->getType()->isFloatTy()) {
+        // MC floating point immediate operands are always encoded with the type
+        // double, so MCOperand::createFPImm takes an argument of type double.
+        // To avoid changes to NaN values that can occur when converting a C++
+        // float to a double, we convert the immediate to a double using
+        // APFloat's architecture-independent FP value manipulation code.
+        bool Ignored;
+        APFloatImm.convert(APFloat::IEEEdouble(), APFloat::rmNearestTiesToEven,
+                           &Ignored);
+      } else
+        assert(Imm->getType()->isDoubleTy() &&
+               "unknown floating point immediate type");
+      MCOp = MCOperand::createFPImm(APFloatImm.convertToDouble());
       break;
     }
     case MachineOperand::MO_GlobalAddress:
       MCOp = lowerSymbolOperand(MO, GetGlobalAddressSymbol(MO));
       break;
     case MachineOperand::MO_ExternalSymbol:
       // The target flag indicates whether this is a symbol for a
       // variable or a function.

llvm/test/CodeGen/WebAssembly/immediates.ll

 ; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-registers | FileCheck %s
 
-; Usually MIPS hosts uses a legacy (non IEEE 754-2008) encoding for NaNs.
-; Tests like `nan_f32` failed in attempt to compare hard-coded IEEE 754-2008
-; NaN value and a legacy NaN value provided by a system.
-; XFAIL: mips-, mipsel-, mips64-, mips64el-
-
 ; Test that basic immediates assemble as expected.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 ; CHECK-LABEL: zero_i32:
 ; CHECK-NEXT: .functype zero_i32 () -> (i32){{$}}
 ; CHECK-NEXT: i32.const $push[[NUM:[0-9]+]]=, 0{{$}}