June 30, 2026
4:35 a.m.
On Tue Jun 30 04:35:42 2026 +0000, Martin Storsjö wrote: > Thanks for looping me in! > This looks like a very reasonable direction to go in overall - this > matches how we've settled on doing feature detection in many other > projects as well (ffmpeg, dav1d etc). > > @mstorsjo, this should build with fairly old headers too. The flags we > use on ARM32 are all quite old, so I don’t think there’s a need to > define them. For ARM64 I’m defining almost everything we use > (`AT_HWCAP2`, and everything newer than the earliest `HWCAP_` flags). > Let me know if additional defines are needed. > In test building with my very old toolchain (which I use because I have > a test device with glibc from 2015 or so), I hit two other missing ones > - we'd need to define `HWCAP_AES` and `HWCAP_CRC32` too. For ARM32 I > didn't hit anything missing with my toolchains though. > Where to get these defines, and whether to redefine them or not, is the > main question that comes up around these... > On ARM, there's another funny discrepancy around them: Normally when > using these, AFAIK, you're supposed to include `<sys/auxv.h>`. This > includes `<bits/hwcap.h>` that contains definitions of them. But they > are also defined in `<asm/hwcap.h>` which we seem to be using here (and > your patches pile on). As far as I've understood, the `<asm/*>` headers > are headers primarily meant for the kernel, that the userspace shouldn't > really be using. > `<asm/hwcap.h>` defines `HWCAP_NEON` as `(1 << 12)` - while > `<bits/hwcap.h>` defines `HWCAP_ARM_NEON` as `4096`. So the defines in > `bits/hwcap.h` have an extra `_ARM` prefix inbetween - and the literal > string they expand to differ. (I don't remember offhand what defines the > BSDs provide here.) > To avoid any potential mismatch between these, as we practically do want > to provide our own fallback defines at least for aarch64, we've settled > on a separate namespace for the defines that we provide, distinct from > the ones from system headers: > https://code.ffmpeg.org/FFmpeg/FFmpeg/src/commit/38b88335f99e76ed89ff3c93f877fdefce736c13/libavutil/aarch64/cpu.c#L27-L34 > There we define them as `HWCAP_AARCH64_CRC32` with an extra `_AARCH64` > inbetween - which no system headers define. > For ARM we used to match the naming from `<asm/hwcap.h>` without > including it - > https://code.ffmpeg.org/FFmpeg/FFmpeg/src/commit/cdae5c3639f4adcd289e643a203d43d4e01d87f5/libavutil/arm/cpu.c#L49-L55. > But since > https://code.ffmpeg.org/FFmpeg/FFmpeg/commit/ced4a6ebc9e7cd92d0ca9b9fb8f9d1013d23cbfa > we've switched to the same naming style as on other architectures - > which happen to coincide with the naming from `<sys/auxv.h>` - which > does cause redefinition warnings as they expand to a different literal > string. In dav1d that's avoided by wrapping the potentially conflicting > ones in an `#ifndef` - https://code.videolan.org/videolan/dav1d/-/blob/master/src/arm/cpu.c?ref_type=heads#L58-L62. > Sorry for the long text... TL;DR: > - Please define `HWCAP_AES` and `HWCAP_CRC32` for aarch64. > - Consider don't relying on `<asm/hwcap.h>` as I think that's linux > kernel specific > - As we're defining these constants unconditionally, consider placing > them in a disjoint namespace to avoid redefinition warnings if system > headers define them with a different literal spelling. Thanks for the quick review, all good points. I've added `HWCAP_AES` and `HWCAP_CRC32` for aarch64. I hadn't realized there were 2 files with the `HWCAP_` defines, it makes sense to stick with just `sys/auxv.h` which we were already including anyway. `asm/hwcap.h` comes from the kernel headers whereas `sys/auxv.h` and `bits/hwcap.h` come from the C library. The redefinition warnings would be annoying, but it seems like the `4096` vs `(1 << 12)` mismatch was just with ARM and not AArch64? I took a look at the kernel AArch64 `asm/hwcap.h`, FreeBSD's `elf.h`, and `bits/hwcap.h` from glibc and musl and they're all consistent. There's always the potential for a mismatch in the future, but as long as everyone stays consistent it seems like there shouldn't be. Maybe we could resolve this in the future on the slim chance it becomes an issue? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11267#note_144513