| Message ID | 20220404091256.683581-1-joel@jms.id.au |
|---|---|
| State | Accepted |
| Headers | show |
| Series | package/zlib-ng: Backport patch to fix linking on powerpc | expand |
On 04/04/2022 11:12, Joel Stanley wrote: > Commit 192dfc68c0e4 ("package/zlib-ng: fix build on powerpc") turned the Peter just backported this one to 2022.02.x... > Power8 optimisations off to fix a build issue. Instead apply a patch > from the develop branch upstream to fix the issue. > > This patch is not yet in a released version of zlib-ng. > > Signed-off-by: Joel Stanley <joel@jms.id.au> Applied to master, thanks. Regards, Arnout > --- > ...rd-for-vec_sumsu-to-prevent-undefine.patch | 27 +++++++++++++++++++ > package/zlib-ng/zlib-ng.mk | 6 ----- > 2 files changed, 27 insertions(+), 6 deletions(-) > create mode 100644 package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch > > diff --git a/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch b/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch > new file mode 100644 > index 000000000000..cc103215de3c > --- /dev/null > +++ b/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch > @@ -0,0 +1,27 @@ > +From 677f56825f7080403e18e57ffe8177f3df290f20 Mon Sep 17 00:00:00 2001 > +From: Nathan Moinvaziri <nathan@nathanm.com> > +Date: Sun, 23 Jan 2022 12:59:01 -0800 > +Subject: [PATCH] Use static keyword for vec_sumsu to prevent undefined > + reference error when g++ linking. > + > +Signed-off-by: Joel Stanley <joel@jms.id.au> > +--- > + arch/power/adler32_power8.c | 2 +- > + 1 file changed, 1 insertion(+), 1 deletion(-) > + > +diff --git a/arch/power/adler32_power8.c b/arch/power/adler32_power8.c > +index 029aa3a84c57..fc4086322efc 100644 > +--- a/arch/power/adler32_power8.c > ++++ b/arch/power/adler32_power8.c > +@@ -44,7 +44,7 @@ > + #include "adler32_p.h" > + > + /* Vector across sum unsigned int (saturate). */ > +-inline vector unsigned int vec_sumsu(vector unsigned int __a, vector unsigned int __b) { > ++static inline vector unsigned int vec_sumsu(vector unsigned int __a, vector unsigned int __b) { > + __b = vec_sld(__a, __a, 8); > + __b = vec_add(__b, __a); > + __a = vec_sld(__b, __b, 4); > +-- > +2.35.1 > + > diff --git a/package/zlib-ng/zlib-ng.mk b/package/zlib-ng/zlib-ng.mk > index 938acd4181a6..fb497b8c11d0 100644 > --- a/package/zlib-ng/zlib-ng.mk > +++ b/package/zlib-ng/zlib-ng.mk > @@ -23,10 +23,4 @@ ifeq ($(BR2_arm),y) > ZLIB_NG_CONF_OPTS += -DWITH_ACLE=1 -DWITH_NEON=1 > endif > > -ifeq ($(BR2_powerpc_power8),y) > -ZLIB_NG_CONF_OPTS += -DWITH_POWER8=ON > -else > -ZLIB_NG_CONF_OPTS += -DWITH_POWER8=OFF > -endif > - > $(eval $(cmake-package))
Arnout, All, On 2022-04-04 22:17 +0200, Arnout Vandecappelle spake thusly: > On 04/04/2022 11:12, Joel Stanley wrote: > >Commit 192dfc68c0e4 ("package/zlib-ng: fix build on powerpc") turned the > Peter just backported this one to 2022.02.x... > >Power8 optimisations off to fix a build issue. Instead apply a patch > >from the develop branch upstream to fix the issue. > > > >This patch is not yet in a released version of zlib-ng. > > > >Signed-off-by: Joel Stanley <joel@jms.id.au> > Applied to master, thanks. Err... Why would we enable the POWER8 optimisations when the target is not a POWER8 ? Backporting the patch seems correct, but the enabling/disabling the optinisations should have stayed, I believe... Regards, Yann E. MORIN. > >--- > > ...rd-for-vec_sumsu-to-prevent-undefine.patch | 27 +++++++++++++++++++ > > package/zlib-ng/zlib-ng.mk | 6 ----- > > 2 files changed, 27 insertions(+), 6 deletions(-) > > create mode 100644 package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch > > > >diff --git a/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch b/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch > >new file mode 100644 > >index 000000000000..cc103215de3c > >--- /dev/null > >+++ b/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch > >@@ -0,0 +1,27 @@ > >+From 677f56825f7080403e18e57ffe8177f3df290f20 Mon Sep 17 00:00:00 2001 > >+From: Nathan Moinvaziri <nathan@nathanm.com> > >+Date: Sun, 23 Jan 2022 12:59:01 -0800 > >+Subject: [PATCH] Use static keyword for vec_sumsu to prevent undefined > >+ reference error when g++ linking. > >+ > >+Signed-off-by: Joel Stanley <joel@jms.id.au> > >+--- > >+ arch/power/adler32_power8.c | 2 +- > >+ 1 file changed, 1 insertion(+), 1 deletion(-) > >+ > >+diff --git a/arch/power/adler32_power8.c b/arch/power/adler32_power8.c > >+index 029aa3a84c57..fc4086322efc 100644 > >+--- a/arch/power/adler32_power8.c > >++++ b/arch/power/adler32_power8.c > >+@@ -44,7 +44,7 @@ > >+ #include "adler32_p.h" > >+ > >+ /* Vector across sum unsigned int (saturate). */ > >+-inline vector unsigned int vec_sumsu(vector unsigned int __a, vector unsigned int __b) { > >++static inline vector unsigned int vec_sumsu(vector unsigned int __a, vector unsigned int __b) { > >+ __b = vec_sld(__a, __a, 8); > >+ __b = vec_add(__b, __a); > >+ __a = vec_sld(__b, __b, 4); > >+-- > >+2.35.1 > >+ > >diff --git a/package/zlib-ng/zlib-ng.mk b/package/zlib-ng/zlib-ng.mk > >index 938acd4181a6..fb497b8c11d0 100644 > >--- a/package/zlib-ng/zlib-ng.mk > >+++ b/package/zlib-ng/zlib-ng.mk > >@@ -23,10 +23,4 @@ ifeq ($(BR2_arm),y) > > ZLIB_NG_CONF_OPTS += -DWITH_ACLE=1 -DWITH_NEON=1 > > endif > >-ifeq ($(BR2_powerpc_power8),y) > >-ZLIB_NG_CONF_OPTS += -DWITH_POWER8=ON > >-else > >-ZLIB_NG_CONF_OPTS += -DWITH_POWER8=OFF > >-endif > >- > > $(eval $(cmake-package)) > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
On Tue, 5 Apr 2022 at 19:55, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > Arnout, All, > > On 2022-04-04 22:17 +0200, Arnout Vandecappelle spake thusly: > > On 04/04/2022 11:12, Joel Stanley wrote: > > >Commit 192dfc68c0e4 ("package/zlib-ng: fix build on powerpc") turned the > > Peter just backported this one to 2022.02.x... > > >Power8 optimisations off to fix a build issue. Instead apply a patch > > >from the develop branch upstream to fix the issue. > > > > > >This patch is not yet in a released version of zlib-ng. > > > > > >Signed-off-by: Joel Stanley <joel@jms.id.au> > > Applied to master, thanks. > > Err... Why would we enable the POWER8 optimisations when the target is > not a POWER8 ? > > Backporting the patch seems correct, but the enabling/disabling the > optinisations should have stayed, I believe... Taking a look at the cmake configuration, there's dozens of options that provide optimisations. It looks like it does detection of features: elseif(BASEARCH_PPC_FOUND) # Common arch detection code if(WITH_ALTIVEC) check_ppc_intrinsics() endif() if(WITH_POWER8) check_power8_intrinsics() endif() if(HAVE_VMX OR HAVE_POWER8_INTRIN) list(APPEND ZLIB_ARCH_HDRS ${ARCHDIR}/power_features.h) list(APPEND ZLIB_ARCH_SRCS ${ARCHDIR}/power_features.c) ... if(WITH_POWER8) if(HAVE_POWER8_INTRIN) add_definitions(-DPOWER8) add_definitions(-DPOWER_FEATURES) add_definitions(-DPOWER8_VSX_ADLER32) add_definitions(-DPOWER8_VSX_CHUNKSET) add_definitions(-DPOWER8_VSX_SLIDEHASH) set(POWER8_SRCS ${ARCHDIR}/adler32_power8.c ${ARCHDIR}/chunkset_power8.c ${ARCHDIR}/slide_hash_power8.c) if("${ARCH}" MATCHES "powerpc64(le)?") add_definitions(-DPOWER8_VSX_CRC32) list(APPEND POWER8_SRCS ${ARCHDIR}/crc32_power8.c) endif() list(APPEND ZLIB_ARCH_SRCS ${POWER8_SRCS}) set_property(SOURCE ${POWER8_SRCS} PROPERTY COMPILE_FLAGS "${POWER8FLAG} ${NOLTOFLAG}") else() set(WITH_POWER8 OFF) endif() The check looks like this. check_power8_intrinsics() { # Check whether features needed by POWER optimisations are available cat > $test.c << EOF #include <sys/auxv.h> int main() { return (getauxval(AT_HWCAP2) & PPC_FEATURE2_ARCH_2_07); } EOF if test $buildpower8 -eq 1 && try $CC -c $CFLAGS -mcpu=power8 $test.c; then HAVE_POWER8_INTRIN=1 echo "Check whether POWER8 instructions are available ... Yes." | tee -a configure.log else HAVE_POWER8_INTRIN=0 echo "Check whether POWER8 instructions are available ... No." | tee -a configure.log fi } So as long as your compiler defines the PPC_FEATURE2_ARCH_2_07 in auxv.h, and supports -mcpu=power8, it will build the code. The application then does runtime checking of auxv to decide if the optimisations should be used. Setting WITH_POWER8=OFF on the command line will skip the checks, and not build in the optimisations. WITH_POWER8=ON has no effect as it's the default and the cmake logic will turn it off if it detects the platform isn't capable. From what I can see, there's a small binary size reduction to be had by forcing Power8 off if your toolchain is power8 capable but you know your platform isn't. I think this is the case for GCC built for ppc64(le) but targeting a different CPU. I suggest we let the zlib-ng build system do its thing to ease the maintenance burden of keeping the flags up to date. I also welcome someone double checking my analysis, as I may have misunderstood something. Cheers, Joel
On Wed, 6 Apr 2022 at 02:05, Joel Stanley <joel@jms.id.au> wrote: > > On Tue, 5 Apr 2022 at 19:55, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > > > Arnout, All, > > > > On 2022-04-04 22:17 +0200, Arnout Vandecappelle spake thusly: > > > On 04/04/2022 11:12, Joel Stanley wrote: > > > >Commit 192dfc68c0e4 ("package/zlib-ng: fix build on powerpc") turned the > > > Peter just backported this one to 2022.02.x... > > > >Power8 optimisations off to fix a build issue. Instead apply a patch > > > >from the develop branch upstream to fix the issue. > > > > > > > >This patch is not yet in a released version of zlib-ng. > > > > > > > >Signed-off-by: Joel Stanley <joel@jms.id.au> > > > Applied to master, thanks. > > > > Err... Why would we enable the POWER8 optimisations when the target is > > not a POWER8 ? > > > > Backporting the patch seems correct, but the enabling/disabling the > > optinisations should have stayed, I believe... > > Taking a look at the cmake configuration, there's dozens of options > that provide optimisations. It looks like it does detection of > features: > I suggest we let the zlib-ng build system do its thing to ease the > maintenance burden of keeping the flags up to date. > > I also welcome someone double checking my analysis, as I may have > misunderstood something. We can ignore my ramblings, because I was looking at the latest 'develop' branch. The cmake configuration in the 2.0.6 release is quite different. I'll send a patch to reinstate the settings we had. Cheers, Joel
diff --git a/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch b/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch new file mode 100644 index 000000000000..cc103215de3c --- /dev/null +++ b/package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch @@ -0,0 +1,27 @@ +From 677f56825f7080403e18e57ffe8177f3df290f20 Mon Sep 17 00:00:00 2001 +From: Nathan Moinvaziri <nathan@nathanm.com> +Date: Sun, 23 Jan 2022 12:59:01 -0800 +Subject: [PATCH] Use static keyword for vec_sumsu to prevent undefined + reference error when g++ linking. + +Signed-off-by: Joel Stanley <joel@jms.id.au> +--- + arch/power/adler32_power8.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/arch/power/adler32_power8.c b/arch/power/adler32_power8.c +index 029aa3a84c57..fc4086322efc 100644 +--- a/arch/power/adler32_power8.c ++++ b/arch/power/adler32_power8.c +@@ -44,7 +44,7 @@ + #include "adler32_p.h" + + /* Vector across sum unsigned int (saturate). */ +-inline vector unsigned int vec_sumsu(vector unsigned int __a, vector unsigned int __b) { ++static inline vector unsigned int vec_sumsu(vector unsigned int __a, vector unsigned int __b) { + __b = vec_sld(__a, __a, 8); + __b = vec_add(__b, __a); + __a = vec_sld(__b, __b, 4); +-- +2.35.1 + diff --git a/package/zlib-ng/zlib-ng.mk b/package/zlib-ng/zlib-ng.mk index 938acd4181a6..fb497b8c11d0 100644 --- a/package/zlib-ng/zlib-ng.mk +++ b/package/zlib-ng/zlib-ng.mk @@ -23,10 +23,4 @@ ifeq ($(BR2_arm),y) ZLIB_NG_CONF_OPTS += -DWITH_ACLE=1 -DWITH_NEON=1 endif -ifeq ($(BR2_powerpc_power8),y) -ZLIB_NG_CONF_OPTS += -DWITH_POWER8=ON -else -ZLIB_NG_CONF_OPTS += -DWITH_POWER8=OFF -endif - $(eval $(cmake-package))
Commit 192dfc68c0e4 ("package/zlib-ng: fix build on powerpc") turned the Power8 optimisations off to fix a build issue. Instead apply a patch from the develop branch upstream to fix the issue. This patch is not yet in a released version of zlib-ng. Signed-off-by: Joel Stanley <joel@jms.id.au> --- ...rd-for-vec_sumsu-to-prevent-undefine.patch | 27 +++++++++++++++++++ package/zlib-ng/zlib-ng.mk | 6 ----- 2 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 package/zlib-ng/0001-Use-static-keyword-for-vec_sumsu-to-prevent-undefine.patch