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