diff mbox series

libpng: Added "--enable-hardware-optimizations" instead of "--enable-arm-neon". Because "--enable-arm-neon" only works for armv7, but doesn't work for aarch64. But in fact neon is also enabled for aarch64 by default.

Message ID 1670899095-3959-1-git-send-email-leimaohui@fujitsu.com
State New
Headers show
Series libpng: Added "--enable-hardware-optimizations" instead of "--enable-arm-neon". Because "--enable-arm-neon" only works for armv7, but doesn't work for aarch64. But in fact neon is also enabled for aarch64 by default. | expand

Commit Message

Maohui Lei (Fujitsu) Dec. 13, 2022, 2:38 a.m. UTC 
Reference to libpng-1.6.38/configure,if enable_hardware_optimizations is
enabled, libpng can judge whether enable enable_arm_neon according
to $host_cpu.
----------------------------------------
$ cat libpng-1.6.38/configure
......
if test ${enable_hardware_optimizations+y}
then :
......
         # allow enabling hardware optimization on any system:
         case "$host_cpu" in
            arm*|aarch64*)
              enable_arm_neon=yes

printf "%s\n" "#define PNG_ARM_NEON_OPT 2" >>confdefs.h
......
----------------------------------------

Signed-off-by: Lei Maohui <leimaohui@fujitsu.com>
---
 meta/recipes-multimedia/libpng/libpng_1.6.38.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Kjellerstedt Dec. 13, 2022, 11:10 a.m. UTC | #1 
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of leimaohui
> Sent: den 13 december 2022 03:38
> To: openembedded-core@lists.openembedded.org
> Cc: Lei Maohui <leimaohui@fujitsu.com>
> Subject: [oe-core][PATCH] libpng: Added "--enable-hardware-optimizations" instead of "--enable-arm-neon". Because "--enable-arm-neon" only works for armv7, but doesn't work for aarch64. But in fact neon is also enabled for aarch64 by default.

The above is a way too long subject. I suggest changing it to:

libpng: Use --enable-hardware-optimizations instead of --enable-arm-neon

And then use the rest of the original subject as the first paragraph 
of the commit message:

Because "--enable-arm-neon" only works for armv7, but doesn't work for
aarch64. But in fact neon is also enabled for aarch64 by default.

> 
> Reference to libpng-1.6.38/configure,if enable_hardware_optimizations is
> enabled, libpng can judge whether enable enable_arm_neon according
> to $host_cpu.
> ----------------------------------------
> $ cat libpng-1.6.38/configure
> ......
> if test ${enable_hardware_optimizations+y}
> then :
> ......
>          # allow enabling hardware optimization on any system:
>          case "$host_cpu" in
>             arm*|aarch64*)
>               enable_arm_neon=yes
> 
> printf "%s\n" "#define PNG_ARM_NEON_OPT 2" >>confdefs.h
> ......
> ----------------------------------------
> 
> Signed-off-by: Lei Maohui <leimaohui@fujitsu.com>
> ---
>  meta/recipes-multimedia/libpng/libpng_1.6.38.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> index dc627203ef..7da71d9d3b 100644
> --- a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> +++ b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> @@ -22,7 +22,7 @@ BINCONFIG = "${bindir}/libpng-config ${bindir}/libpng16-config"
>  inherit autotools binconfig-disabled pkgconfig
> 
>  # Work around missing symbols
> -EXTRA_OECONF:append:class-target = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off", d)}"
> +EXTRA_OECONF:append:class-target = " --enable-hardware-optimizations=on "

Remove the space before the trailing quote.

> 
>  PACKAGES =+ "${PN}-tools"
> 
> --
> 2.25.1

//Peter
Alexandre Belloni Dec. 13, 2022, 7:28 p.m. UTC | #2 
Hello,

This fails on the autobuilders:

https://autobuilder.yoctoproject.org/typhoon/#/builders/107/builds/3913/steps/11/logs/stdio

On 13/12/2022 10:38:15+0800, leimaohui wrote:
> Reference to libpng-1.6.38/configure,if enable_hardware_optimizations is
> enabled, libpng can judge whether enable enable_arm_neon according
> to $host_cpu.
> ----------------------------------------
> $ cat libpng-1.6.38/configure
> ......
> if test ${enable_hardware_optimizations+y}
> then :
> ......
>          # allow enabling hardware optimization on any system:
>          case "$host_cpu" in
>             arm*|aarch64*)
>               enable_arm_neon=yes
> 
> printf "%s\n" "#define PNG_ARM_NEON_OPT 2" >>confdefs.h
> ......
> ----------------------------------------
> 
> Signed-off-by: Lei Maohui <leimaohui@fujitsu.com>
> ---
>  meta/recipes-multimedia/libpng/libpng_1.6.38.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> index dc627203ef..7da71d9d3b 100644
> --- a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> +++ b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> @@ -22,7 +22,7 @@ BINCONFIG = "${bindir}/libpng-config ${bindir}/libpng16-config"
>  inherit autotools binconfig-disabled pkgconfig
>  
>  # Work around missing symbols
> -EXTRA_OECONF:append:class-target = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off", d)}"
> +EXTRA_OECONF:append:class-target = " --enable-hardware-optimizations=on "
>  
>  PACKAGES =+ "${PN}-tools"
>  
> -- 
> 2.25.1
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174506): https://lists.openembedded.org/g/openembedded-core/message/174506
> Mute This Topic: https://lists.openembedded.org/mt/95637494/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Andre McCurdy Dec. 13, 2022, 9:48 p.m. UTC | #3 
On Mon, Dec 12, 2022 at 6:38 PM leimaohui <leimaohui@fujitsu.com> wrote:
>
> Reference to libpng-1.6.38/configure,if enable_hardware_optimizations is
> enabled, libpng can judge whether enable enable_arm_neon according
> to $host_cpu.
> ----------------------------------------
> $ cat libpng-1.6.38/configure
> ......
> if test ${enable_hardware_optimizations+y}
> then :
> ......
>          # allow enabling hardware optimization on any system:
>          case "$host_cpu" in
>             arm*|aarch64*)
>               enable_arm_neon=yes
>
> printf "%s\n" "#define PNG_ARM_NEON_OPT 2" >>confdefs.h
> ......
> ----------------------------------------
>
> Signed-off-by: Lei Maohui <leimaohui@fujitsu.com>
> ---
>  meta/recipes-multimedia/libpng/libpng_1.6.38.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> index dc627203ef..7da71d9d3b 100644
> --- a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> +++ b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> @@ -22,7 +22,7 @@ BINCONFIG = "${bindir}/libpng-config ${bindir}/libpng16-config"
>  inherit autotools binconfig-disabled pkgconfig
>
>  # Work around missing symbols
> -EXTRA_OECONF:append:class-target = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off", d)}"
> +EXTRA_OECONF:append:class-target = " --enable-hardware-optimizations=on "

It's difficult (impossible?) to automatically detect whether an ARM
target supports NEON at build time, so this looks unlikely to work.
Testing for NEON either needs to happen at run time or the information
needs to be passed from the build system as an explicit configure
option (ie the original approach here).

If you don't want to pass NEON options to 64bit ARM targets then add
an over-ride which only applies to 32bit ARM targets.

Or in this case, if the configure script is wrong to try to enable
NEON for 64bit ARM targets, patch the configure script and submit the
patch upstream.

>  PACKAGES =+ "${PN}-tools"
>
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#174506): https://lists.openembedded.org/g/openembedded-core/message/174506
> Mute This Topic: https://lists.openembedded.org/mt/95637494/3619030
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [armccurdy@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Maohui Lei (Fujitsu) Dec. 15, 2022, 2:30 a.m. UTC | #4 
> It's difficult (impossible?) to automatically detect whether an ARM target supports
> NEON at build time, so this looks unlikely to work.
> Testing for NEON either needs to happen at run time or the information needs to
> be passed from the build system as an explicit configure option (ie the original
> approach here).
> 
> If you don't want to pass NEON options to 64bit ARM targets then add an
> over-ride which only applies to 32bit ARM targets.
> 
> Or in this case, if the configure script is wrong to try to enable NEON for 64bit ARM
> targets, patch the configure script and submit the patch upstream.

I got it.
In fact, I submitted this patch because I met a conflict error when I enable multilib on aarch64. The conflict reason is that my lib32-libpng enabled NEON but libpng(64bit) didn't enable NEON although NEON is enabled for all aarch64 by default.
But it looks like it is not a good way to added " --enable-hardware-optimizations=on " for all target. 
So how about to update my patch to V2 as the following:

-EXTRA_OECONF:append:class-target = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off", d)}"
+EXTRA_OECONF:append:class-target = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", " ", d)}"
+EXTRA_OECONF:append:aarch64 = " --enable-hardware-optimizations=on "

Best regards
Lei





> -----Original Message-----
> From: Andre McCurdy <armccurdy@gmail.com>
> Sent: Wednesday, December 14, 2022 5:49 AM
> To: Lei, Maohui <leimaohui@fujitsu.com>
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [oe-core][PATCH] libpng: Added "--enable-hardware-optimizations"
> instead of "--enable-arm-neon". Because "--enable-arm-neon" only works for
> armv7, but doesn't work for aarch64. But in fact neon is also enabled for aarch64
> by default.
> 
> On Mon, Dec 12, 2022 at 6:38 PM leimaohui <leimaohui@fujitsu.com> wrote:
> >
> > Reference to libpng-1.6.38/configure,if enable_hardware_optimizations
> > is enabled, libpng can judge whether enable enable_arm_neon according
> > to $host_cpu.
> > ----------------------------------------
> > $ cat libpng-1.6.38/configure
> > ......
> > if test ${enable_hardware_optimizations+y}
> > then :
> > ......
> >          # allow enabling hardware optimization on any system:
> >          case "$host_cpu" in
> >             arm*|aarch64*)
> >               enable_arm_neon=yes
> >
> > printf "%s\n" "#define PNG_ARM_NEON_OPT 2" >>confdefs.h ......
> > ----------------------------------------
> >
> > Signed-off-by: Lei Maohui <leimaohui@fujitsu.com>
> > ---
> >  meta/recipes-multimedia/libpng/libpng_1.6.38.bb | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> > b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> > index dc627203ef..7da71d9d3b 100644
> > --- a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> > +++ b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
> > @@ -22,7 +22,7 @@ BINCONFIG = "${bindir}/libpng-config
> ${bindir}/libpng16-config"
> >  inherit autotools binconfig-disabled pkgconfig
> >
> >  # Work around missing symbols
> > -EXTRA_OECONF:append:class-target = "
> ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on",
> "--enable-arm-neon=off", d)}"
> > +EXTRA_OECONF:append:class-target = " --enable-hardware-optimizations=on "
> 
> It's difficult (impossible?) to automatically detect whether an ARM target supports
> NEON at build time, so this looks unlikely to work.
> Testing for NEON either needs to happen at run time or the information needs to
> be passed from the build system as an explicit configure option (ie the original
> approach here).
> 
> If you don't want to pass NEON options to 64bit ARM targets then add an
> over-ride which only applies to 32bit ARM targets.
> 
> Or in this case, if the configure script is wrong to try to enable NEON for 64bit ARM
> targets, patch the configure script and submit the patch upstream.
> 
> >  PACKAGES =+ "${PN}-tools"
> >
> > --
> > 2.25.1
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#174506):
> > https://lists.openembedded.org/g/openembedded-core/message/174506
> > Mute This Topic: https://lists.openembedded.org/mt/95637494/3619030
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
> > [armccurdy@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
diff mbox series

Patch

diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
index dc627203ef..7da71d9d3b 100644
--- a/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
+++ b/meta/recipes-multimedia/libpng/libpng_1.6.38.bb
@@ -22,7 +22,7 @@  BINCONFIG = "${bindir}/libpng-config ${bindir}/libpng16-config"
 inherit autotools binconfig-disabled pkgconfig
 
 # Work around missing symbols
-EXTRA_OECONF:append:class-target = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off", d)}"
+EXTRA_OECONF:append:class-target = " --enable-hardware-optimizations=on "
 
 PACKAGES =+ "${PN}-tools"