Closed Bug 1501542 Opened 3 years ago Closed 2 years ago

Implement CheckARMSupport for Android

Categories

(NSS :: Libraries, enhancement, P1)

ARM
Android
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(2 files, 1 obsolete file)

Android 18+ (4.3+) has getauxval function, so we should use it when this function is available even if Android.
Android 4.3 or later has getauxval, so we should use it to detect ARM features.

Also, current CPU detection has a bug to detect NEON.  AT_HWCAP2 doesn't return
NEON bit.
I investigate more for sources/android/cpufeatures/cpu-features.c

Although looking cpu-features.c in sources/android/cpufeatures/cpu-features.c, some devices may return 0 by getauxval (most is that this function isn't implemented).  If this situation, cpu-features.c reads /proc/self/auxv instead.

So if getauxval returns non-zero value, cpu-features doesn't use this fallback in cpu-features.c.

So when getauxval returns non-zero, it is valid.  And if getauxval returns invalid value that is non-zero, cpu-features.c cannot detect valid cpu features.

Could you check sources/android/cpufeatures/cpu-features.c in NDK?
Flags: needinfo?(franziskuskiefer)
So it means that NDK document says getauxval may return 0 even if some features are supported.
Android 4.3 or later has getauxval, so we can use it to detect ARM features.

Also, current CPU detection has a bug to detect NEON.  AT_HWCAP2 doesn't return
NEON bit.  So it should use AT_HWCAP instead of AT_HWCAP2.  But some old device such as Nexus 4 doesn't return valid value for getauxval(AT_HWCAP).  So we don't trust return value of AT_HWCAP on Android.

Since AT_HWCAP2 is implemented in newer devices, cpu-features in Android NDK doesn't have fallback such as AT_HWCAP case.  So we can trust it.

Also, if toolchain / system's default is NEON, compiler defines __ARM_NEON__ and/or __ARM_NEON (like x86's __SSE2__ defines), so if it is defined, we always
allow NEON.

Depends on D11430
To compile ARM's NEON code, compiler may require -mfpu=neon.

Actually, since Gecko always turn on NEON (Bug 1469790), it already uses -mfpu.
But tier-3 platform such as Linux/armeabi doesn't set -mfpu=neon as default.
So it might require this command line option.
Attachment #9021077 - Attachment is obsolete: true
Flags: needinfo?(franziskuskiefer)

Makoto Kato - Are you still planning to revise the attached patches per Franziskus' feedback? If so, note that you'll need a new reviewer. Needinfo me in that case and I'll find you one. Thanks!

Status: NEW → ASSIGNED
Flags: needinfo?(m_kato)
Keywords: stalled
Priority: -- → P1

(In reply to J.C. Jones [:jcj] (he/him) from comment #6)

Makoto Kato - Are you still planning to revise the attached patches per Franziskus' feedback? If so, note that you'll need a new reviewer. Needinfo me in that case and I'll find you one. Thanks!

Although I set reviewer to you, who is better for ARM issue? I have a plan that I would like to add ARM specific optimization such as bug 1152625.

Flags: needinfo?(m_kato) → needinfo?(jjones)

(Note: Working on finding one!)

Looks like Martin is your best bet, and he's already here. Clearing my ni?

Flags: needinfo?(jjones)
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Hardware: Unspecified → ARM
Resolution: --- → FIXED
Target Milestone: --- → 3.44
You need to log in before you can comment on or make changes to this bug.