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.
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.
(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).
(In reply to Piotr Kubaj from comment #2) I approve the cpu.c change if you want to go ahead and unbreak the build.
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
(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.
(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.
(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.
(In reply to Piotr Kubaj from comment #7) Just disable it for the time being, please.
Created attachment 206708 [details] v2 Is this ok? The src/libFLAC/cpu.c is still needed (compiling without it fails because of no getauxval).
Yes, this is ok. I have also sent some feedback to the flac-dev mailing list.
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
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