sabnzbd / sabctools

C implementations of functions for use within SABnzbd
GNU General Public License v2.0
21 stars 11 forks source link

Build failures on Synology #37

Closed Safihre closed 2 years ago

Safihre commented 2 years ago

I tried building on the SynoCommunity platform for all the Synology platforms and some more stuff came up: https://github.com/Safihre/spksrc/actions/runs/2027059583

We have a nice selection:

jcfp commented 2 years ago

hi3535 is probably this soc by hisilicon.

Safihre commented 2 years ago

@animetosho For hi3535 it keeps failing, it seems to be a Cortex A9, which should be 32bit ARMv7. Although our code doesn't detect it as ARMv7. And something about -march=armv8-a+crc. https://github.com/Safihre/spksrc/runs/5658113916?check_suite_focus=true#step:8:30

animetosho commented 2 years ago

Although our code doesn't detect it as ARMv7

...what I don't like about platform.machine()...

Are these cross compiles? If so, platform.machine() is going to be completely wrong.
It's also entirely possible that the platform name doesn't start with 'armv7', but I don't have a comprehensive list of how the naming works.
(if supporting older systems, you may need to check for ARMv6; I presumed that only ARMv7 and later mattered)

And something about -march=armv8-a+crc.

The flag requires GCC >= 4.9.0 - the list given by the error seems to indicate it's using a 4.8.x version?!
I don't anything about SynoCommunity, but if it's stuck on really old build platforms (4.9.0 was released April 2014 - around the same time as Python 3.4.0), the only way to really solve this is to detect whether the compiler supports the flag in setup.py.

I think -std=c++11 was first accepted in GCC 4.7, so the compiler not supporting that would be older.

Safihre commented 2 years ago

Yeah, they are ancient. They are provided by Synology as the toolchain to use for a specific model, so usually the toolchain that was available at the moment the model was released. That's sometimes indeed a decade ago. But somehow it all still works.. I'll see what I can do to fix things!

Safihre commented 2 years ago

Almost got it. Turns out SynoCommunity has all kind of tricks for the cross-compiling so the right flags can be supplied through CFLAGS etc and we don't have to detect it in setup.py. One special case seems to be 88f6281, there it errors out with fatal error: sys/auxv.h: No such file or directory. Have you seen that before? If it's too exotic, I'm just listing that chip as unsupported.

jcfp commented 2 years ago

@Safihre from what I see on debian/ubuntu that file should be provided by the libc cross building support. The short description for all libc6-dev-*-cross packages' is GNU C Library: Development Libraries and Header Files (for cross-compiling):

$ apt-file search sys/auxv.h
dietlibc-dev: /usr/include/diet/sys/auxv.h
libc6-dev: /usr/include/x86_64-linux-gnu/sys/auxv.h
libc6-dev-amd64-cross: /usr/x86_64-linux-gnu/include/sys/auxv.h
libc6-dev-arm64-cross: /usr/aarch64-linux-gnu/include/sys/auxv.h
libc6-dev-armel-cross: /usr/arm-linux-gnueabi/include/sys/auxv.h
libc6-dev-armhf-cross: /usr/arm-linux-gnueabihf/include/sys/auxv.h
libc6-dev-hppa-cross: /usr/hppa-linux-gnu/include/sys/auxv.h
libc6-dev-i386: /usr/include/sys/auxv.h
libc6-dev-i386-cross: /usr/i686-linux-gnu/include/sys/auxv.h
libc6-dev-m68k-cross: /usr/m68k-linux-gnu/include/sys/auxv.h
libc6-dev-mips-cross: /usr/mips-linux-gnu/include/sys/auxv.h
libc6-dev-mips64-cross: /usr/mips64-linux-gnuabi64/include/sys/auxv.h
libc6-dev-mips64el-cross: /usr/mips64el-linux-gnuabi64/include/sys/auxv.h
libc6-dev-mips64r6-cross: /usr/mipsisa64r6-linux-gnuabi64/include/sys/auxv.h
libc6-dev-mips64r6el-cross: /usr/mipsisa64r6el-linux-gnuabi64/include/sys/auxv.h
libc6-dev-mipsel-cross: /usr/mipsel-linux-gnu/include/sys/auxv.h
libc6-dev-mipsn32-cross: /usr/mips64-linux-gnuabin32/include/sys/auxv.h
libc6-dev-mipsn32el-cross: /usr/mips64el-linux-gnuabin32/include/sys/auxv.h
libc6-dev-mipsn32r6-cross: /usr/mipsisa64r6-linux-gnuabin32/include/sys/auxv.h
libc6-dev-mipsn32r6el-cross: /usr/mipsisa64r6el-linux-gnuabin32/include/sys/auxv.h
libc6-dev-mipsr6-cross: /usr/mipsisa32r6-linux-gnu/include/sys/auxv.h
libc6-dev-mipsr6el-cross: /usr/mipsisa32r6el-linux-gnu/include/sys/auxv.h
libc6-dev-powerpc-cross: /usr/powerpc-linux-gnu/include/sys/auxv.h
libc6-dev-ppc64-cross: /usr/powerpc64-linux-gnu/include/sys/auxv.h
libc6-dev-ppc64el-cross: /usr/powerpc64le-linux-gnu/include/sys/auxv.h
libc6-dev-riscv64-cross: /usr/riscv64-linux-gnu/include/sys/auxv.h
libc6-dev-s390x-cross: /usr/s390x-linux-gnu/include/sys/auxv.h
libc6-dev-sh4-cross: /usr/sh4-linux-gnu/include/sys/auxv.h
libc6-dev-sparc64-cross: /usr/sparc64-linux-gnu/include/sys/auxv.h
libc6-dev-x32-cross: /usr/x86_64-linux-gnux32/include/sys/auxv.h
libc6.1-dev-alpha-cross: /usr/alpha-linux-gnu/include/sys/auxv.h
libklibc-dev: /usr/lib/klibc/include/sys/auxv.h
musl-dev: /usr/include/x86_64-linux-musl/sys/auxv.h

Might want to ask the synology overlords why that particular file is missing for one platform (and blacklist in the meantime?).

animetosho commented 2 years ago

I think autotools works by testing compiler capabilities by running the flags to see if it fails. It might be a nicer approach than relying on special SynoCommunity tricks as it should work everywhere, but does mean you'd have to do detection.

'auxv.h' is used for detecting processor capabilities on ARM.
Searching for '88F6281' leads me to this, which shows an ARMv5 processor. NEON requires ARMv7, though the code should compile for ARMv6.
I don't know what ARMv5 had (v6 was released ~20 years ago), but not having CPU feature detection doesn't sound unusual. Regardless, ARMv5 probably shouldn't be considered to be "ARM" here, since none of the ARM specific features will work on it. So in setup.py, you could try to detect ARMv5 and exclude that from the IS_ARM list.

Safihre commented 2 years ago

Thanks! Working on this Python-autotools and made some good progress. I was wondering 2 things though:

EDIT: Based on this list do I understand correctly mfpu should be best to first check for neon-fp-armv8, if not found then neon-vfpv4 and then neon?

EDIT 2: If -march=armv8-a+crc is not available, is there any benefit in having -march=armv8-a? Or just skip the whole -march in case there's no +crc? Since this is specific for the crc_arm.cc file.

animetosho commented 2 years ago

The code will compile without the flags, however the features won't be enabled. Obviously, the flags won't be accepted if you're not compiling for ARM (v6 or later), so you at least need to check for that (though brute-force checking parameters will also work).

The CRC feature was added in ARMv8.0 as an optional extension (was made mandatory in ARMv8.1), so you still need to provide the flag to enable CRC acceleration, even on ARMv8.

ARMv8 32-bit is largely the same as ARMv7 (ARMv8 is mostly known for the new 64-bit ISA (AArch64 or ARM64), which is a completely different architecture to 32-bit ARM), so there's little reason to target it if compiling for 32-bit ARM. Since CRC was only added in ARMv8 though, you have to specifically specify it there.

ARMv7 supported CPUs without an FPU (floating point unit). Although we don't do any floating point math in yEnc, SIMD (even integer SIMD) is implemented on the FPU. So for NEON, we need to compile for a chip with an FPU with NEON capabilities. There's no particular reason to also request VFP functionality because it isn't used.

I'm not sure why -mfpu=fp-armv8 was needed. ARMv8 mandates an FPU, so one would think that supplying -march=armv8-a would enable it. Perhaps the compiler was targeting a processor without an FPU, then got confused when it was told to target ARMv8, because ARMv8 without an FPU isn't valid. It seems like explicitly setting an FPU is enough to fix that.