Re: [PATCH] cmake: Default to PNG_ARM_NEON=off for arm targets

Martin Storsjö <[email protected]> Fri, 14 Oct 2022 23:21:07 +0300 (EEST)
Newsgroups gmane.comp.graphics.png.devel
Message-ID <[email protected]>
  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-205577930-1665778869=:1659
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8BIT

On Fri, 14 Oct 2022, John Bowler wrote:

> >So, John, to conclude your point: do you agree that Martin's patch does the
> right thing, or do you not? And if not, then what is the correct solution in
> your opinion?
> 
> I don't know enough about cmake to comment on that; I was responding to
> Martin's observation at the start of this thread.  He said:
> 
> >This matches the configure script - unless the user has requested anything,
> no hardware optimizations are autoenabled.
> 
> And that is certainly not the behavior of configure.  If the user just
> types:
> 
> > .../configure
> 
> (I.e. no arguments, "not requesting anything") then the compiler checks are
> run in pngpriv.h and the optimisations are turned on if the compiler
> supports them because the relevant _OPT symbol is not defined.  This applies
> to NEON, VSX and MIPS; INTEL is broken because of the spurious check in a C
> file on an AM_CONDITIONAL...  I unborked it and it works fine.
> 
> So there is disagreement between what you say cmake does and what Martin
> says it does.  Your description is the correct behavior so assuming Martin
> is wrong everything seems fine.  It's easy to check; just do configure/cmake
> then make; if it's working correctly then the "arm" subdirectory will
> contain .o files and "nm" will show they have the right symbols.

Sorry, this statement was indeed wrong. The CMake build system gave a 
somewhat different image of how things work - initially, I was under the 
impression that the CMake build files were more endorsed and canonical 
than they now in hindsight clearly are.

With the configure build system, with no custom flags set, you do get NEON 
enabled as long as the compiler defaults to it enabled (i.e. 
automatiacally picked up by pngpriv.h) - i.e. everywhere on aarch64, and 
on armv7 for OSes where it's part of the baseline (i.e. iOS and Windows, 
but not on most linux distributions).

With the CMake build, with no custom flags set, you do get NEON enabled on 
aarch64, but not on armv7, not even on OSes where it's part of the 
baseline. (Prior to the CMake patch that was applied, the CMake build 
failed totally for those targets, unless flags were set manually.)

With respect to enabling NEON, I've identified these discrepancies between 
configure and CMake:

- CMake defaults to PNG_HARDWARE_OPTIMIZATIONS=ON, essentially equivalent
   to --enable-hardware-optimizations in configure

- With PNG_HARDWARE_OPTIMIZATIONS disabled in CMake, the build is done
   with -DPNG_ARM_NEON_OPT=0, i.e. pngpriv.h won't automatically pick up
   anything

- With PNG_HARDWARE_OPTIMIZATIONS enabled, but for an arch where it
   doesn't hard-enable it (i.e. arm, contrary to aarch64), it will leave
   out the arm sources entirely, and build with -DPNG_ARM_NEON_OPT=0 - i.e.
   no autodetection thanks to pngpriv.h

- And on top of this, for arm it used to default to automatically setting
   -DPNG_ARM_NEON_CHECK_SUPPORTED, but it no longer does.

I.e. the CMake build is much more rigid and essentially wants to decide 
mostly everything about how to build it upfront.

// Martin

--8323329-205577930-1665778869=:1659
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline


--8323329-205577930-1665778869=:1659
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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

--8323329-205577930-1665778869=:1659--