|Summary:||audio/flac: fix build on powerpc64|
|Product:||Ports & Packages||Reporter:||Piotr Kubaj <pkubaj>|
|Component:||Individual Port(s)||Assignee:||Christian Weisgerber <naddy>|
|Severity:||Affects Only Me||CC:||linimon, naddy|
Description Piotr Kubaj 2019-08-15 00:22:18 UTC
Created attachment 206567 [details] patch FreeBSD doesn't support getauxval(), it has elf_aux_info() instead. Use this to fix build on powerpc64. Since flac seems to add VSX by default on POWER9, add a non-default option to enable VSX.
Comment 1 Christian Weisgerber 2019-08-15 16:57:22 UTC
I agree with the libFLAC/cpu.c diff. I don't understand the intended purpose of the VSX option. In libFLAC/stream_encoder.c, the POWER9 and POWER8 routines are only called if cpuinfo.ppc.arch_3_00 and cpuinfo.ppc.arch_2_07, respectively, are true. So the VSX code is compiled on all powerpc64 machines, but only called at runtime on those that support the VSX extensions.
Comment 2 Piotr Kubaj 2019-08-15 17:58:48 UTC
(In reply to Christian Weisgerber from comment #1) Ah, you're right about VSX. BTW, the patch for elf_aux_info is being upstreamed now (in a slightly better version).
Comment 3 Christian Weisgerber 2019-08-15 21:10:31 UTC
(In reply to Piotr Kubaj from comment #2) I approve the cpu.c change if you want to go ahead and unbreak the build.
Comment 4 Piotr Kubaj 2019-08-16 15:04:16 UTC
Ah, I forgot, -mvsx is necessary: lpc_intrin_vsx.c: In function 'FLAC__lpc_compute_autocorrelation_intrin_power8_vsx_lag_16': lpc_intrin_vsx.c:94:7: warning: implicit declaration of function 'vec_vsx_ld'; did you mean 'vec_vslh'? [-Wimplicit-function-declaration] 94 | d0 = vec_vsx_ld(0, base); | ^~~~~~~~~~ | vec_vslh
Comment 5 Christian Weisgerber 2019-08-16 21:33:35 UTC
(In reply to Piotr Kubaj from comment #4) Well, if -mvsx is required, an option doesn't make sense either since the build would fail with it off. So should -mvsx be added on powerpc64? Or does this cause the compiler to produce VSX code outside of the intrinsics that would then fail to run on some powerpc64 machines? I can simply disable assembly optimizations on powerpc64. It is far more important for the port to build (as a dependency) than to be tuned for speed. Also, I just tried it without asm on amd64 and encoding is plenty fast.
Comment 6 Piotr Kubaj 2019-08-18 02:59:57 UTC
(In reply to Christian Weisgerber from comment #5) The problem is that vec_vsx_ld isn't visible without -mvsx. vec_vsx_ld is needed, unless we disable VSX altogether (--disable-vsx). We could pass -mvsx by default, but I'm not sure whether VSX would be autodisabled on POWER older than POWER8 so that flac would work. Basically, we have 3 options: a) add -mvsx for everyone on powerpc64, b) disable VSX for everyone on powerpc64, c) make VSX optional via option.
Comment 7 Piotr Kubaj 2019-08-18 03:02:38 UTC
(In reply to Piotr Kubaj from comment #6) And I'd rather not disable unconditionally that may improve performance on hardware that supports it. Users get new hardware in order to use new features, not to disable them.
Comment 8 Christian Weisgerber 2019-08-18 20:15:19 UTC
(In reply to Piotr Kubaj from comment #7) Just disable it for the time being, please.
Comment 9 Piotr Kubaj 2019-08-19 19:59:40 UTC
Created attachment 206708 [details] v2 Is this ok? The src/libFLAC/cpu.c is still needed (compiling without it fails because of no getauxval).
Comment 10 Christian Weisgerber 2019-08-19 21:34:17 UTC
Yes, this is ok. I have also sent some feedback to the flac-dev mailing list.
Comment 11 commit-hook 2019-08-28 19:27:31 UTC
A commit references this bug: Author: pkubaj Date: Wed Aug 28 19:26:57 UTC 2019 New revision: 510095 URL: https://svnweb.freebsd.org/changeset/ports/510095 Log: audio/flac: fix build on powerpc64 FreeBSD doesn't support getauxval(), it has elf_aux_info() instead. Use this to fix build on powerpc64. Disable VSX to not use VSX-related functions. PR: 239870 Approved by: linimon (mentor), naddy (maintainer) Differential Revision: https://reviews.freebsd.org/D21284 Changes: head/audio/flac/Makefile head/audio/flac/files/patch-src_libFLAC_cpu.c
Comment 12 commit-hook 2019-08-28 20:22:37 UTC
A commit references this bug: Author: naddy Date: Wed Aug 28 20:21:45 UTC 2019 New revision: 510099 URL: https://svnweb.freebsd.org/changeset/ports/510099 Log: Use proper autoconf test for getauxval() and elf_aux_info(). Based on work by Jeremie Courreges-Anglas for OpenBSD. PR: 239870 Changes: head/audio/flac/Makefile head/audio/flac/files/patch-configure.ac head/audio/flac/files/patch-src_libFLAC_cpu.c