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

On Tue, 11 Oct 2022, Cosmin Truta wrote:

> Hello, and thank you for your contribution. There are already other errors
> in the existing code in pngpriv.h.
> 
> For example, earlier on, PNG_ARM_NEON_OPT is set to 2 exactly under the
> conditions that you're mentioning. (See the lines 130-133.) The real bug is
> behind the conflict of PNG_ARM_NEON_OPT being equal to 2 on one hand, and
> PNG_ARM_NEON_CHECK_SUPPORTED being enabled on the other hand. These two
> conflicting conditions should be in fact mutually exclusive.

Sure. In this case, as far as I can see, we can either move the #undef of 
PNG_ARM_NEON_CHECK_SUPPORTED into this ifdef block which sets 
PNG_ARM_NEON_OPT to 2 - resolving the conflicting macros right away, or 
check for this in the build systems.

As far as I can see, the cmake build defaults to adding 
-DPNG_ARM_NEON_CHECK_SUPPORTED for any arm arch, while with autoconf, it 
only does that if the user explicitly requested --enable-arm-neon=check. 
So to resolve the inconsistency at that level, we'd need to add a check in 
CMake to check whether __ARM_NEON__ or __ARM_NEON are defined.

Which one of those do you prefer?

> The same error exists in the hardware optimizations of other processors
> also, and it needs to be fixed globally.

Sounds plausible, but I'd like to stick to fixing things for the arm 
targets in this patch.

> I propose introducing a preprocessor assertion, something like this:
> 
> #if (PNG_ARM_NEON_OPT > 1) && \
>     defined(PNG_ARM_NEON_CHECK_SUPPORTED)
> #error Assertion failed blah blah blah
> #endif

Sure, this sounds reasonable to add.

// Martin

_______________________________________________
png-mng-implement mailing list
png-mng-implement-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/png-mng-implement