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

On Tue, Oct 11, 2022 at 8:47 AM Martin Storsjö <[email protected]> wrote:

>  #     define PNG_ARM_NEON_OPT 2
> +#     undef PNG_ARM_NEON_CHECK_SUPPORTED
>  #  else
>  #     define PNG_ARM_NEON_OPT 0
>  #  endif
> --
>
> You fundamentally misunderstand how it works.  PNG_ARM_NEON_OPT is a
*setting*, PNG_ARM_NEON_CHECK_SUPPORTED is an *option*.  Both are recorded
in pnglibconf.h which is part of the public API; so perhaps neither should
be there, but they are.  pnglibconf.h is machine generated by
scripts/pnglibconf.dfa and the PNG_ARM_NEON_OPT settings are documented in
that file.   The PowerPC ones are in there too, but somehow there is a MIPS
option in pngpriv.h which does not seem to be generated.

So the compile time check requires -UPNG_ARM_NEON_OPT; setting
PNG_ARM_NEON_OPT to *anything *basically takes over responsibility.
Normally this is wrong!  The reason the whole shebang is the is to support
various broken build systems where the compiler didn't set things.
Typically that's because the compiler didn't actually support the relevant
hardware, NEON in this case, and the implementation was hand-crafted
machine code or assembler.

I suggest at this point simplifying it; remove both NEON and VSX option
handling, remove PNG_ARM_NEON_OPT and remove the runtime API; by remove I
mean simply ignore it (since it is a png_set_option API).   See what
breaks; most likely nothing.  For sure remove the code from pngpriv.h; ARM,
MIPS and POWERPC alike.

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