Bug 274086 - REGRESSION(278410@main): Fails to build with clang: error: _Float16 is not supported on this target
Summary: REGRESSION(278410@main): Fails to build with clang: error: _Float16 is not su...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-13 07:29 PDT by Alberto Garcia
Modified: 2024-05-15 12:16 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Garcia 2024-05-13 07:29:20 PDT
I have reproduced this problem on at least i386 and s390x.

x86_64 works fine.

                        ----------------------------
Source/WTF/wtf/simde/arm/neon.h:8808:11: error: _Float16 is not supported on this target
  typedef _Float16 simde_float16;
          ^
Source/WTF/wtf/simde/arm/neon.h:34339:47: error: _Float16 is not supported on this target
    return simde_vceq_f16(a, simde_vdup_n_f16(SIMDE_FLOAT16_VALUE(0.0)));
                                              ^
Source/WTF/wtf/simde/arm/neon.h:8975:38: note: expanded from macro 'SIMDE_FLOAT16_VALUE'
  #define SIMDE_FLOAT16_VALUE(value) SIMDE_FLOAT16_C(value)
                                     ^
Source/WTF/wtf/simde/arm/neon.h:8813:55: note: expanded from macro 'SIMDE_FLOAT16_C'
    #define SIMDE_FLOAT16_C(value) HEDLEY_STATIC_CAST(_Float16, (value))
                                                      ^
                        ----------------------------
Comment 1 Michael Catanzaro 2024-05-14 08:47:49 PDT
I can reproduce on s390x. This blocks updating to 2.45.1/2.45.2.

At first I thought "surely wtf/simde/arm is not supposed to be built on non-ARM architectures" but actually it is; it's intended to provide the ARM APIs on non-ARM architectures. It seems the preprocessor tests in simde-f16.h will need some work.
Comment 2 Michael Catanzaro 2024-05-14 09:25:40 PDT
The problem is this branch https://github.com/simd-everywhere/simde/blob/f046ab773733f09edaadec30345b592dfe85368e/simde/simde-f16.h#L86 is being taken improperly, for unknown reasons. The condition is very complex.

I don't see this on i686. Maybe the i386 vs. i686 microarchitecture difference is relevant.
Comment 3 Michael Catanzaro 2024-05-14 12:37:03 PDT
(In reply to Michael Catanzaro from comment #2)
> I don't see this on i686. Maybe the i386 vs. i686 microarchitecture
> difference is relevant.

I've been checking the GCC [1][2] and Clang [3] manuals, and the difference must be SSE2 instruction set. Debian probably does not enable it. Fedora does.

[1] https://gcc.gnu.org/onlinedocs/gcc/Floating-Types.html
[2] https://gcc.gnu.org/onlinedocs/gcc/Half-Precision.html
[3] https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
Comment 4 Alberto Garcia 2024-05-14 14:02:05 PDT
> the difference must be SSE2 instruction set. Debian probably does not enable it

Oh, I see, the i386 baseline in Debian is i686, which does not include SSE:

https://wiki.debian.org/ArchitectureSpecificsMemo#i386-1

When this started to cause problems with WebKitGTK I disabled anything
that would produce such code. The idea was that all PC users should
switch to a 64-bit OS, and anyone who was still running the 32-bit
version was probably running very old hardware so compatibility was
more important than performance.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930935
Comment 5 Michael Catanzaro 2024-05-14 14:30:49 PDT
I think it's broken only when using Clang rather than GCC. I've created a pull request at https://github.com/simd-everywhere/simde/pull/1182. There is a *lot* of upstream CI and I'm not expecting it all to pass; we'll see how it goes.

Berto, give this a try please:

diff --git a/Source/WTF/wtf/simde/arm/neon.h b/Source/WTF/wtf/simde/arm/neon.h
index 1ecd8cf565f1..b6d0fdf4f5d1 100644
--- a/Source/WTF/wtf/simde/arm/neon.h
+++ b/Source/WTF/wtf/simde/arm/neon.h
@@ -8785,8 +8785,7 @@ SIMDE_BEGIN_DECLS_
       defined(SIMDE_X86_AVX512FP16_NATIVE) || \
       (defined(SIMDE_ARCH_X86_SSE2) && HEDLEY_GCC_VERSION_CHECK(12,0,0)) || \
       (defined(SIMDE_ARCH_AARCH64) && HEDLEY_GCC_VERSION_CHECK(7,0,0) && !defined(__cplusplus)) || \
-      ((defined(SIMDE_ARCH_X86) || defined(SIMDE_ARCH_AMD64)) && SIMDE_DETECT_CLANG_VERSION_CHECK(15,0,0)) || \
-      (!(defined(SIMDE_ARCH_X86) || defined(SIMDE_ARCH_AMD64)) && SIMDE_DETECT_CLANG_VERSION_CHECK(6,0,0))) || \
+      ((defined(SIMDE_ARCH_X86_SSE2) || defined(SIMDE_ARCH_AMD64)) && SIMDE_DETECT_CLANG_VERSION_CHECK(15,0,0))) || \
       defined(SIMDE_ARCH_RISCV_ZVFH)
     /* We haven't found a better way to detect this.  It seems like defining
     * __STDC_WANT_IEC_60559_TYPES_EXT__, then including float.h, then
Comment 6 Alberto Garcia 2024-05-15 03:06:35 PDT
> Berto, give this a try please

Tested in Debian experimental i386, it builds and works fine (on a VM at least).
Comment 7 Michael Catanzaro 2024-05-15 09:23:46 PDT
Pull request: https://github.com/WebKit/WebKit/pull/28597
Comment 8 EWS 2024-05-15 12:16:15 PDT
Committed 278821@main (3b302c530534): <https://commits.webkit.org/278821@main>

Reviewed commits have been landed. Closing PR #28597 and removing active labels.