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

On Tue, 11 Oct 2022, Cosmin Truta wrote:

> On Tue, Oct 11, 2022 at 4:30 PM Cosmin Truta <[email protected]> wrote:
>>
>> Actually, I think I'm having a change of mind:
>>
>> A developer may enable the __ARM_NEON__ codegen at compile time, but
>> that is not necessarily an assurance that the intended target machine
>> (to be used at run time) is actually Neon-enabled.
>
> D'oh! I take this back. I think that you, Martin, are correct to say
> that __ARM_NEON__ is an indication that the compiler may liberally
> insert ARM Neon code anywhere, which further means that checking for
> ARM Neon hardware availability at run time is futile.

Exactly, these defines imply that the compiler may generate such code 
anywhere

>> On the other hand, I do agree that the configure script and the cmake
>> file should be consistent in their behavior.
>
> ... but I do stand behind this other statement.

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

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

Which one of these two would you prefer here?

The thirdly, we can add the assert that you requested, to check that in 
the end, PNG_ARM_NEON_OPT and PNG_ARM_NEON_CHECK_SUPPORTED don't 
contradict each other.

// Martin