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