Skip to content

[wasm] Fix build failure due to lack of _Float16 support #72891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2024
Merged

[wasm] Fix build failure due to lack of _Float16 support #72891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion include/swift/Runtime/SwiftDtoa.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@
#define SWIFT_DTOA_BINARY16_SUPPORT 1
#endif

/// Does this platform support needs to pass _Float16 as a float in
/// C function?
#ifndef SWIFT_DTOA_PASS_FLOAT16_AS_FLOAT
// Windows does not define FLT16_MAX even though it supports _Float16 as argument.
# if (!defined(FLT16_MAX) || defined(__wasm__)) && !defined(_WIN32)
# define SWIFT_DTOA_PASS_FLOAT16_AS_FLOAT 1
# else
# define SWIFT_DTOA_PASS_FLOAT16_AS_FLOAT 0
# endif
#endif

//
// IEEE 754 Binary32 support (also known as "single-precision")
//
Expand Down Expand Up @@ -239,7 +250,7 @@ extern "C" {

#if SWIFT_DTOA_BINARY16_SUPPORT
size_t swift_dtoa_optimal_binary16_p(const void *, char *dest, size_t length);
#if defined FLT16_MAX
#if !SWIFT_DTOA_PASS_FLOAT16_AS_FLOAT
// If `_Float16` is defined, provide this convenience wrapper.
size_t swift_dtoa_optimal_binary16(_Float16, char *dest, size_t length);
#endif
Expand Down
12 changes: 10 additions & 2 deletions stdlib/public/core/Runtime.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,14 +350,22 @@ internal struct _Buffer72 {
}

#if !((os(macOS) || targetEnvironment(macCatalyst)) && arch(x86_64))
#if arch(wasm32)
// Note that this takes a Float32 argument instead of Float16, because clang
// doesn't have _Float16 on all platforms yet.
@available(SwiftStdlib 5.3, *)
typealias _CFloat16Argument = Float32
#else
@available(SwiftStdlib 5.3, *)
typealias _CFloat16Argument = Float16
#endif

@available(SwiftStdlib 5.3, *)
@_silgen_name("swift_float16ToString")
internal func _float16ToStringImpl(
_ buffer: UnsafeMutablePointer<UTF8.CodeUnit>,
_ bufferLength: UInt,
_ value: Float16,
_ value: _CFloat16Argument,
_ debug: Bool
) -> Int

Expand All @@ -370,7 +378,7 @@ internal func _float16ToString(
_internalInvariant(MemoryLayout<_Buffer32>.size == 32)
var buffer = _Buffer32()
let length = buffer.withBytes { (bufferPtr) in
_float16ToStringImpl(bufferPtr, 32, value, debug)
_float16ToStringImpl(bufferPtr, 32, _CFloat16Argument(value), debug)
}
return (buffer, length)
}
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/runtime/SwiftDtoa.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ static size_t nan_details(char *dest, size_t len, int negative, int quiet, uint6


#if SWIFT_DTOA_BINARY16_SUPPORT
#if defined FLT16_MAX
#if !SWIFT_DTOA_PASS_FLOAT16_AS_FLOAT
// Format a C `_Float16`
size_t swift_dtoa_optimal_binary16(_Float16 d, char *dest, size_t length) {
return swift_dtoa_optimal_binary16_p(&d, dest, length);
Expand Down
13 changes: 12 additions & 1 deletion stdlib/public/stubs/Stubs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,21 @@ static locale_t getCLocale() {
#endif
#endif // SWIFT_STDLIB_HAS_LOCALE

#if SWIFT_DTOA_PASS_FLOAT16_AS_FLOAT
using _CFloat16Argument = float;
#else
using _CFloat16Argument = _Float16;
#endif

SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_API
__swift_ssize_t swift_float16ToString(char *Buffer, size_t BufferLength,
_Float16 Value, bool Debug) {
_CFloat16Argument Value, bool Debug) {
#if SWIFT_DTOA_PASS_FLOAT16_AS_FLOAT
__fp16 v = Value;
return swift_dtoa_optimal_binary16_p(&v, Buffer, BufferLength);
#else
return swift_dtoa_optimal_binary16_p(&Value, Buffer, BufferLength);
#endif
}

SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_API
Expand Down
4 changes: 4 additions & 0 deletions test/stdlib/PrintFloat16.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ PrintTests.test("Printable_Float16") {
expectEqual(Float16.infinity.debugDescription, "inf")
expectEqual((-Float16.infinity).debugDescription, "-inf")

// Platforms without float 16 argument passing can cause NaNs to be changed
// while being passed.
#if !arch(wasm32)
for bitPattern in (0x7c01 as UInt16) ... 0x7fff {
expectEqual(Float16(bitPattern: bitPattern).description, "nan")
expectEqual(Float16(bitPattern: 0x8000 | bitPattern).description, "nan")
Expand All @@ -144,6 +147,7 @@ PrintTests.test("Printable_Float16") {
expectEqual(Float16(bitPattern: bitPattern).debugDescription, expected)
expectEqual(Float16(bitPattern: 0x8000 | bitPattern).debugDescription, "-\(expected)")
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does that happen? Surely Float32 can represent at least as many NaN values as Float16?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Float32 can represent all NaN patterns that can be representable as Float16. However, sNaN values are converted to qNaN while roundtripping Float16 -> Float32 -> Float16 for argument passing on platforms that don't have native Float16 support.
The actual conversion happens in a builtin that is used by swift_float16ToString to truncate Float32 -> Float16: https://github.com/llvm/llvm-project/blob/1107b47dcd145518c7b811bf10e2b848782b0478/compiler-rt/lib/builtins/fp_trunc_impl.inc#L111

Therefore the check fails on WebAssembly unfortunatley.

check failed at /home/katei/ghq/work.katei.dev/swift-source/swift/test/stdlib/PrintFloat16.swift, line 151
first:  "nan(0xff)" (of type Swift.String)
second: "snan(0xff)" (of type Swift.String)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a minimum reproducible code for the implicit bit pattern change.

#include <stdio.h>
#include <stdint.h>

void dump_bits(void *d, size_t size) {
  unsigned char *ptr = (unsigned char *) d;
  for (int i = size - 1; i >= 0; i--) {
    unsigned int mask = 1 << (8 - 1);
    unsigned char byte = ptr[i];
    do {
      putchar(mask & byte ? '1' : '0');
    } while(mask >>= 1);
  }
  putchar('\n');
}

int main(void) {
  uint16_t snan_bits = 32000;
  union {
    __fp16 f;
    uint16_t i;
  } u = { .i = snan_bits };
  dump_bits(&u.f, sizeof(__fp16));

  float casted = (float)u.f;
  dump_bits(&casted, sizeof(casted));

  __fp16 casted_back = (__fp16)casted;
  dump_bits(&casted_back, sizeof(casted_back));

  return 0;
}
$ clang -target wasm32-unknown-wasi check.c -o check.wasm
$ wasmtime check.wasm
0111110100000000
01111111101000000000000000000000
0111111100000000

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, sNaN values are converted to qNaN while roundtripping Float16 -> Float32 -> Float16 for argument passing on platforms that don't have native Float16 support.

I wouldn't claim to be an expert (@stephentyrone is the man to talk to here), but that sounds like a bug.

Copy link
Member Author

@kateinoigakukun kateinoigakukun Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert here too, but as far as I investigated, the root cause seems the LLVM default lowering behavior for half type that passes around half type as float. Due to the LLVM lowering, Float16 is represented as f32 at Wasm level and sNaN values are converted to qNaN while extending bitPattern of Float16(bitPattern:) to Float32. (I said the conversion happens while roundtripping f16 -> f32 -> f16 in printing, but I found it also happens more primitive Float16(bitPattern:) casting)

I feel changing NaN payload pattern while bit-width extension seems not a strange behavior. According to x64 manual, some instructions that convert float to double (e.g. CVTSS2SD) return qNaN when the source is sNaN1. And also Wasm does similar canonicalization for fp operations including truncation and promotion against f32/f642.

If we really want to guarantee bitwise equivalence between the given bitPattern and the resulted Float16's bitPattern, we have three options:

  1. Change compiler-rt's __extendhfsf2 to hold NaN payload patterns
  2. Change LLVM to enable softPromoteHalfType for Wasm targets to pass around half type as i16 at Wasm level
  3. Change IRGen to use i16 to represent Builtin.FPIEEE16 instead of half at LLVM IR level for such archs

But I'm not sure it's worth paying such effort only for sNaN Float16 on platforms without native support, but I'd like to hear @stephentyrone 's opinion.

Footnotes

  1. Table D-11. CVTPS2PD and CVTSS2SD, Intel® 64 and IA-32 Architectures Software Developer’s Manual Combined Volumes: 1, 2A, 2B, 2C, 2D, 3A, 3B, 3C, 3D, and 4 ↩

  2. https://webassembly.github.io/spec/core/exec/numerics.html#nan-propagation ↩

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the correct behavior. A conversion is required to silence NaNs per 754.

#endif
}

Expand Down