Re: [PATCH v2] arm: Don't require a runtime check function for targets with unconditional NEON

Cosmin Truta <[email protected]> Tue, 11 Oct 2022 22:13:13 +0300
Newsgroups gmane.comp.graphics.png.devel
Message-ID <CAAoVtZwXwcHU98BKq0Xgij3=U5HizjXh6sdG_E-UbdTP1nDavg@mail.gmail.com>
On Tue, Oct 11, 2022 at 8:25 PM John Bowler
<[email protected]> wrote:
>> While the user could override this when configuring the build, I though it
>> could be avoided - on iOS and Windows on armv7, NEON is part of the
>> platform baseline, so the compiler is free to use NEON anywhere in code
>> generated from the C code, so no runtime check should be needed. The
>> compiler signals this, that NEON code generation is enabled
>> unconditionally, by defining __ARM_NEON__ (or __ARM_NEON).
>
> Nope; you are fixing a cmake bug by breaking an API.  Sure, removing the API is fine, but just breaking it because of a simple failure to update CMakeLists.txt is clearly wrong.

Actually, Martin is correct. I was inclined to say the same thing as
John, earlier today; but no.

If __ARM_NEON is defined, then the compiler will liberally insert ARM
Neon code anywhere in the optimized C code. No API is being broken, as
this is merely an internal change in behavior. If the compiler (or,
rather, the user who drives the compiler) is already assuming that ARM
Neon hardware is readily available on the compiled target, there is
absolutely no point whatsoever to start checking for said
availability.

Earlier today I was talking about adding assertions in pngpriv.h, but
I think it's better to implement them in the ARM-specific code, right
besides the ARM64 checks:

--- arm/arm_init.c
+++ arm/arm_init.c
@@ -37,6 +37,9 @@
 #  if defined(__aarch64__) || defined(_M_ARM64)
      /* ARM Neon is expected to be unconditionally available on ARM64. */
 #    error "PNG_ARM_NEON_CHECK_SUPPORTED must not be defined on this platform"
+#  elif defined(__ARM_NEON__) || defined(__ARM_NEON)
+     /* ARM Neon is already expected by the compiler to be available. */
+#    error "PNG_ARM_NEON_CHECK_SUPPORTED must not be defined under
the current compiler options"
 #  elif defined(__linux__)
 #    define PNG_ARM_NEON_FILE "contrib/arm-neon/linux.c"
 #  else