-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[FMV][AArch64] Add initial AT_HWCAP3 / AT_HWCAP4 support #161595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ea5fdd8
to
6e571ee
Compare
@@ -190,3 +190,19 @@ | |||
#ifndef HWCAP2_CSSC | |||
#define HWCAP2_CSSC (1UL << 34) | |||
#endif | |||
|
|||
#ifndef AT_HWCAP3 | |||
#if defined(__linux__) || defined(__ANDROID__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't seem to care for non linux/android for other macros like AT_HWCAP and AT_HWCAP2 above. I couldn't find the values 38 and 39 for AT_HWCAP3/AT_HWCAP4 can you shed some light?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't seem to care for non linux/android for other macros like AT_HWCAP and AT_HWCAP2 above.
AT_HWCAP and AT_HWCAP2 are so old that i assume we don't support compiling with a libc that doesn't have them. 3 and 4 require relatively new libcs though.
(it might be worth saying that in a code comment, so a future reader knows they can remove this in 2028 or whenever.)
also: linux is true for ANDROID, so there's no need to explicitly mention ANDROID here.
I couldn't find the values 38 and 39 for AT_HWCAP3/AT_HWCAP4 can you shed some light?
yeah, i'm confused by that... probably worth explicitly saying (in code or #if) if this is for FreeBSD or whatever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't seem to care for non linux/android for other macros like AT_HWCAP and AT_HWCAP2 above. I couldn't find the values 38 and 39 for AT_HWCAP3/AT_HWCAP4 can you shed some light?
Only because *BSD's include sys/auxv.h where as for Linux that does not appear to be the case. AT_HWCAP / AT_HWCAP2 are guaranteed to be defined for the *BSD's if the functionality (elf_aux_info) is there, not so for 3 and 4. Those values are for *BSD's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AT_HWCAP and AT_HWCAP2 are so old that i assume we don't support compiling with a libc that doesn't have them.
getauxval() was added in glibc 2.16. AArch64 support was added in glibc 2.17. AT_HWCAP2 was added in glibc 2.18.
There is some room for simplifying some of this stuff since the expectation is that any system that is on AArch64 has some of these bits included unlike with 32-bit ARM if you go back far enough.
a995ba2
to
ec44d70
Compare
@@ -12,10 +12,14 @@ void CONSTRUCTOR_ATTRIBUTE __init_cpu_features(void) { | |||
|
|||
unsigned long hwcap = getauxval(AT_HWCAP); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit: at this point, you don't need the locals and could just assign straight into the struct. not a regression though.)
@@ -190,3 +190,20 @@ | |||
#ifndef HWCAP2_CSSC | |||
#define HWCAP2_CSSC (1UL << 34) | |||
#endif | |||
|
|||
// Linux and BSD values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: having // Linux
and // BSD
on each of the four lines might be clearer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i realize there's no single "bsd" define we can use, otherwise i'd have suggested that and a #error instead.
actually, looking at my local checkouts ... it seems like these are only in FreeBSD atm anyway?
Add support for AT_HWCAP3 / AT_HWCAP4 which is supported by glibc, musl, Android and FreeBSD 15/-current. Stop using sys/ifunc.h as libgcc has done. This is more portable as older glibc will not have the hwcap3/4 fields.
ec44d70
to
6d0f0a4
Compare
We're seeing some build failures after this patch. It seems you're assuming there is some minium libc version you can assume, but AFAIK there is no such minimum stated in LLVM. I think libcxx has some minimum, but that's very much separate from compiler-rt and the builtins. I think this should probably be reverted until there's some community agreement about updating (in this case specifying at all) what's required. [273/417](25) Building C object CMakeFiles/clang_rt.builtins-aarch64.dir/cpu_model/aarch64.c.o
FAILED: CMakeFiles/clang_rt.builtins-aarch64.dir/cpu_model/aarch64.c.o
/Users/swarming/b/s/w/ir/x/w/llvm_build/./bin/clang --target=aarch64-unknown-linux-gnu --sysroot=/Users/swarming/b/s/w/ir/x/w/cipd/linux -DHAS_ASM_LSE -DVISIBILITY_HIDDEN -D_LIBATOMIC_USE_PTHREAD -I/Users/swarming/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/../../../third-party/siphash/include --target=aarch64-unknown-linux-gnu -O2 -g -DNDEBUG -std=gnu11 -fno-lto -nostdinc++ -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -DCOMPILER_RT_HAS_FLOAT16 -MD -MT CMakeFiles/clang_rt.builtins-aarch64.dir/cpu_model/aarch64.c.o -MF CMakeFiles/clang_rt.builtins-aarch64.dir/cpu_model/aarch64.c.o.d -o CMakeFiles/clang_rt.builtins-aarch64.dir/cpu_model/aarch64.c.o -c /Users/swarming/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/cpu_model/aarch64.c
In file included from /Users/swarming/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/cpu_model/aarch64.c:79:
/Users/swarming/b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/lib/builtins/cpu_model/aarch64/fmv/getauxval.inc:19:35: error: use of undeclared identifier 'hwcap'
19 | __init_cpu_features_constructor(hwcap | _IFUNC_ARG_HWCAP, &arg);
| ^~~~~
1 error generated. |
arg._hwcap2 = getauxval(AT_HWCAP2); | ||
arg._hwcap3 = getauxval(AT_HWCAP3); | ||
arg._hwcap4 = getauxval(AT_HWCAP4); | ||
__init_cpu_features_constructor(hwcap | _IFUNC_ARG_HWCAP, &arg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is still referencing hwcap
but the variable is no longer defined anywhere.
Actually, maybe the error is just that there's still references left? In any case, I'd appreciate it if you could take a look. |
Yes, I have a diff to revert the part that broke the build. |
Add support for AT_HWCAP3 / AT_HWCAP4 which is supported by glibc, musl,
Android and FreeBSD 15/-current.
Stop using sys/ifunc.h as libgcc has done. This is more portable as
older glibc will not have the hwcap3/4 fields.