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

On Tue, Oct 11, 2022 at 7:07 PM John Bowler
<[email protected]> wrote:

> PNG_ARM_NEON_OPT is a *setting*, PNG_ARM_NEON_CHECK_SUPPORTED is an *option*.

Reading this statement reminds me how insanely over-engineered the
libpng build configuration system is. What other library or codec
comes even close? (This is not a rhetorical question, and I'm not
trying to be sarcastic here; just asking about best practices employed
elsewhere.)

> I suggest at this point simplifying it

I vote yes in principle to pretty much any statement that has the word
"simplify" in it. Not for libpng-1.6.x (because I'd rather not cause
gratuitous workflow disruptions for any of our users), but for the
future versions.

> 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.

I'm ok with this if we don't break the current behaviour; otherwise it
should be left for after 1.6.x.

> 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.

The cmake build has been changing its behaviour all the time, while
the configure build has been quite stable.

I agree with the suggestion to change the cmake build behaviour in
order to make it follow the configure build behaviour.

Sincerely,
Cosmin