-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[compiler-rt][AArch64][Android] Use getauxval on Android. #102979
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
__getauxval is a libgcc function that doesn't exist on Android.
Calling getauxval on Android is fine since: The original motivation is broken as compiler-rt does call getauxval a lot from other places as of today. |
static _Bool has_sme(void) { | ||
return __getauxval(AT_HWCAP2) & HWCAP2_SME; | ||
} | ||
static _Bool has_sme(void) { return GETAUXVAL(AT_HWCAP2) & HWCAP2_SME; } |
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 think that I would rather that we inline the use of __getauxval
. getauxval
is a GNU "standard" interface, but was introduced in glibc only in 2.16. I don't know if anyone is targeting an older glibc, but we can do something like:
static _Bool has_sme(void) { return GETAUXVAL(AT_HWCAP2) & HWCAP2_SME; } | |
static _Bool has_sme(void) { | |
#if defined(__GLIBC__) && !__GLIBC_PREREQ(2,16) | |
unsigned long int auxval = __getauxval(AT_HWCAP2); | |
#else | |
unsigned long int auxval = getauxval(AT_HWCAP2); | |
#endif | |
return auxval & HWCAP2_SME; | |
} |
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.
AArch64 support for getauxval just added in 2.17.
Compiler-rt uses getauxval with glibc in multiple places(SLE_Atomic, FMV, Sanitizers), so I don't see the point why use __getauxval
only here.
I don't think we need to worry about such a platform that has SME
while doesn't have getauxval
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.
It avoids the macros and allows us to have a single platform agnostic implementation. Over time, we would drop the use of the underscore variant of the function.
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.
Sorry for the delay, I didn't realise this PR was still waiting for review.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/3256 Here is the relevant piece of the build log for the reference
|
This reverts commit 51e2e12. Reason for revert: The llvm bug fix llvm/llvm-project#102979 has been rolled into Chrome in https://chromium-review.googlesource.com/5921462. Original change's description: > Do not enable libyuv_use_sme for is_android > > Revert the changes to libyuv.gni in commit dfa279f. > > The linker error "undefined symbol: __getauxval" referenced by > sme-abi-init.c:26 on Android, previously reported in > https://libyuv.g-issues.chromium.org/issues/359006069#comment2, has not > been fixed yet. See > https://chromium-review.googlesource.com/c/chromium/src/+/5918245?tab=checks. > > Change-Id: I94bd243e2863b9c316909f63f757fd95ec55dc18 > Reviewed-on: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/5917455 > Reviewed-by: Frank Barchard <[email protected]> Bug: 359006069 Change-Id: Ic801c1bcb65894fdfe718ba6454669c8623a2e15 Reviewed-on: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/5935026 Reviewed-by: Frank Barchard <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: George Steed <[email protected]>
__getauxval is a libgcc function that doesn't exist on Android.