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

On Tue, Oct 11, 2022 at 5:50 PM Martin Storsjö <[email protected]> wrote:

> >> On the other hand, I do agree that the configure script and the cmake
> >> file should be consistent in their behavior.
>
> Sure. Unless the user specified anything, currently CMake sets
> PNG_ARM_NEON to "check" (and thus defines PNG_ARM_NEON_CHECK_SUPPORTED),
> while the configure script doesn't (but it should be easy to change
> configure to do the same).

Unless somebody else says otherwise, my response is that,
historically, the cmake file has been changing its behavior in all
kinds of ways, while the configure script was more stable. I'd rather
change the CMake file to follow the configure script.

On top of that, there's the issue of libpng users who use neither the
configure script, nor the cmake file, but rather, take the source code
and embed it into their own build systems. Very notable examples:
Android AOSP and Chromium.

My point here is that if the source code can configure itself without
a necessary aid from the external configure or cmake scripts, that'd
be the best.

For your testing, you can see how the makefiles are working:

./ci/ci_autotools.sh
./ci/ci_cmake.sh
./ci/ci_legacy.sh

> If we want to avoid the inconsistency, either pngpriv.h will undef
> PNG_ARM_NEON_CHECK_SUPPORTED when it realizes that it indeed doesn't need
> any check - this is quite simple to achieve. (Essentially what my patch
> does, but the undef can be moved closer to the "#define PNG_ARM_NEON_OPT
> 2" to avoid inconsistencies over longer ranges.)

I like this. It will work correctly, without requiring configure or
cmake to work correctly.

On top of that...

> Or we can have the build
> scripts try to compile a small file to check whether __ARM_NEON__ and/or
> __ARM_NEON is defined, and default to "on" instead of "check". This is a
> bit more of code (as the logic needs to be duplicated in both cmake and
> configure).

This is also something good to have, although -- again, in my personal
opinion only -- is secondary, but still useful.

It's secondary, meaning that the source code itself should be able to
do the autodetection. And it's still useful, meaning that an
inconsistency between the user requesting the "check" option should be
told, by the means of a warning, or even an error, that the "check" is
being disabled.

I wish I had more expertise on this, but I don't. These are just ideas
off the top of my head.

I wonder what's the best industry practice, or, at least, what is done
in other projects that also implement this kind of optimization in
this kind of configurable way.

Sincerely,
Cosmin


_______________________________________________
png-mng-implement mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/png-mng-implement