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

--===============5805776627516098982==
Content-Type: multipart/alternative; boundary="00000000000018111205eb028267"

--00000000000018111205eb028267
Content-Type: text/plain; charset="UTF-8"

On Fri, Oct 14, 2022 at 4:34 AM Cosmin Truta <[email protected]> wrote:

> On Thu, Oct 13, 2022 at 9:48 PM John Bowler
> <[email protected]> wrote:
> > I double checked.  It is exactly as I said; if you do not pass *any*
> command line arguments to configure you get the relevant (target) CPU
> optimizations compiled in so long as the compiler supports them.  (The
> latter check happens in pngpriv.h, which, as I said, is probably the wrong
> place.)
>
> The same goes for the CMake file. I actually tested it.
>

Good.  It might be useful to flag any attempt to use these options apart,
maybe, from the "turn all optimisations off".  Forcing the hardware stuff
on is very dangerous and likely to lead to compile time failure with
different compilers.  It's only useful for developers working on hardware
stuff who don't, yet, have adequate compiler support.  In the past people
have misunderstood and assumed it was necessary to pass an --enable option
to configure to turn it on, failing to realise that it happens
automagically by default.

IRC the *--enable-hardware-optimizations* configure option that is right at
the start of this block of configure stuff was intended to replace all the
other options in a future release.

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

>It is worth introducing a new API function that informs the user about the
available optimizations.

No.  What is an app going to do with that information other than something
that causes problems?  It's just feature creep/code bloat.

For configure:

.../configure --disable-hardware-optimizations # no hardware specific files
should be built, or if they are built they should be empty
.../configure --enable-hardware-optimizations  # the platform specific
hardware should be built, but it may fail with non-gcc compilers (try
clang...)
.../configure # should *always* work, and should build the hardware opts if
the previous step was able to

--00000000000018111205eb028267
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">On Fri, Oct 14, 2022 at 4:34 AM Cosmin Tr=
uta &lt;<a href=3D"mailto:[email protected]">[email protected]</a>&gt; wrote:=
<br></div><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex">On Thu, Oct 13, 2022 at 9:48 PM John Bowler<br>
&lt;<a href=3D"mailto:[email protected]" target=3D"_blank">j=
[email protected]</a>&gt; wrote:<br>
&gt; I double checked.=C2=A0 It is exactly as I said; if you do not pass *a=
ny* command line arguments to configure you get the relevant (target) CPU o=
ptimizations compiled in so long as the compiler supports them.=C2=A0 (The =
latter check happens in pngpriv.h, which, as I said, is probably the wrong =
place.)<br>
<br>
The same goes for the CMake file. I actually tested it.<br></blockquote><di=
v><br></div><div>Good.=C2=A0 It might be useful to flag any attempt to use =
these options apart, maybe, from the &quot;turn all optimisations off&quot;=
.=C2=A0 Forcing the hardware stuff on is very dangerous and likely to lead =
to compile time failure with different compilers.=C2=A0 It&#39;s only usefu=
l for developers working on hardware stuff who don&#39;t, yet, have adequat=
e compiler support.=C2=A0 In the past people have misunderstood and assumed=
 it was necessary to pass an --enable option to configure to turn it on, fa=
iling to realise that it happens automagically by default.</div><div><br></=
div><div>IRC the <b>--enable-hardware-optimizations</b> configure option th=
at is right at the start of this block of configure stuff was intended to r=
eplace all the other options in a future release.</div><div><br></div><div>=
&gt;So, John, to conclude your point: do you agree that Martin&#39;s patch =
does the right thing, or do you not? And if not, then what is the correct s=
olution in your opinion?<br></div><div><br></div><div>I don&#39;t know enou=
gh about cmake to comment on that; I was responding to Martin&#39;s observa=
tion at the start of this thread.=C2=A0 He said:</div><div><br></div><div>&=
gt;This matches the configure script - unless the user has requested anythi=
ng, no hardware optimizations are autoenabled.</div><div><br></div><div>And=
 <b>that </b>is certainly <b>not</b> the behavior of configure.=C2=A0 If th=
e user just types:</div><div><br></div><div>&gt; .../configure</div><div><b=
r></div><div>(I.e. no arguments, &quot;not requesting anything&quot;) then =
the compiler checks are run in pngpriv.h and the optimisations are turned o=
n if the compiler supports them because the relevant _OPT symbol is not def=
ined.=C2=A0 This applies to NEON, VSX and MIPS; INTEL is broken because of =
the spurious check in a C file on an AM_CONDITIONAL...=C2=A0 I unborked it =
and it works fine.<br></div><div><br></div><div>So there is disagreement be=
tween what you say cmake does and what Martin says it does.=C2=A0 Your desc=
ription is the correct behavior so assuming Martin is wrong everything seem=
s fine.=C2=A0 It&#39;s easy to check; just do configure/cmake then make; if=
 it&#39;s working correctly then the &quot;arm&quot; subdirectory will cont=
ain .o files and &quot;nm&quot; will show they have the right symbols.<br><=
/div><div><br></div><div>&gt;It is worth introducing a new API function tha=
t informs the user about the available optimizations.</div><div><br></div><=
div>No.=C2=A0 What is an app going to do with that information other than s=
omething that causes problems?=C2=A0 It&#39;s just feature creep/code bloat=
.</div><div><br></div><div>For configure:</div><div><br></div><div>.../conf=
igure --disable-hardware-optimizations # no hardware specific files should =
be built, or if they are built they should be empty</div><div>.../configure=
 --enable-hardware-optimizations=C2=A0 # the platform specific hardware sho=
uld be built, but it may fail with non-gcc compilers (try clang...)</div><d=
iv>.../configure # should *always* work, and should build the hardware opts=
 if the previous step was able to</div><div><br></div><div><br></div><div><=
br></div><div><br></div></div></div>

--00000000000018111205eb028267--


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


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

--===============5805776627516098982==--