Closed Bug 1571613 Opened Last month Closed 26 days ago

Add Power ISA SIMD detection to mozglue

Categories

(Core :: mozglue, enhancement)

Other
Linux
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: spectre, Assigned: spectre)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 5 obsolete files)

It's Time To Make AltiVec Great Again

This adds VMX, VSX and VSX-3 detection to mozglue. Unfortunately, while the altivec string in /proc/cpuinfo is invariant for VMX, VSX is not yet exposed there, so we check for processor strings a la MIPS for the time being.

Prerequisite for some other work I will be prepping to land.

Assignee: nobody → spectre

Wouldn't it be better to base the flags on getauxval(AT_HWCAP{,2}) output rather than to parse /proc/cpuinfo?

I guess I could have an XP_LINUX check in there for that, and then fall back to /proc/cpuinfo if not available so that it would still build on a non-Linux (i.e., BSD, etc.) OS.

Attachment #9083216 - Attachment is obsolete: true
Attached patch ppc-simd-detect.patch (obsolete) — Splinter Review

I think we can rely on <sys/auxv.h> also for the CPU feature flags (defined in <bits/hwcap.h>). ARCH_3_00 is available since glibc 2.23 (released 2016-02-19), so I've rather added a check for it, but maybe it's old enough.

I'm fine with your concept if you want to submit that instead.

Attachment #9083531 - Attachment is obsolete: true

Dan, are you submitting it, or am I? I want to try to get some other stuff in but I need this first.

Dan and I discussed in E-mail and I will submit in the interests of time since it's blocking other work.

Attachment #9083571 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attached patch BSD support (obsolete) — Splinter Review

/proc/cpuinfo doesn't expose powerpc* features on FreeBSD and NetBSD while OpenBSD removed /proc filesystem entirely. FreeBSD 12+ has a function similar to getauxval(). After this patch bug 817042 can be retired. Can you integrate it into the review on phabricator?

Also, /proc fallback should probably be replaced with compile-time detection based on -maltivec (aka __ALTIVEC__), -msvx (aka __VSX__), -mpower9-vector (aka __POWER9_VECTOR__) flags.
https://github.com/llvm/llvm-project/blob/llvmorg-8.0.1/clang/lib/Basic/Targets/PPC.cpp#L160-L175

Landry, does OpenBSD support detecting VSX and/or VSX3 at runtime?
Martin, does NetBSD support detecting VSX and/or VSX3 at runtime?

I don't have a way of testing on the *BSDs (for that matter, I'm not aware that anything but FreeBSD supports POWER8+). This is blocking additional work and I'd prefer someone who can test it do that in a followup. I need this to land any of the Altivec stuff I'm working on; I can't land it until the build support is in.

As far as compile vs runtime, the "Mozilla way" seems to be runtime detection, not compile-time detection, or at least that's what everything else in mozglue/build is doing. For little vs big endian PPC in future patches, that will be compile-time, since that distinction is already compile-time in Firefox. I'll defer to :glandium if that is no longer the case.

Duplicate of this bug: 817042

(In reply to Cameron Kaiser [:spectre] from comment #12)

I'd prefer someone who can test it do that in a followup.

I can't test as part of Firefox (doesn't build, anyway) but FreeBSD bits extracted into standalone sample worked fine on ref12-ppc64.freebsd.org. NetBSD bits were tested on x86_64 after replacing CPU_ALTIVEC with HW_NCPU (can't use powerpc* macro on non-powerpc*). OpenBSD has similar code in devel/sdl package.

In other words, if you want the perfect testing then it won't happen for years. During that time living with broken /proc fallback doesn't seem desirable.

the "Mozilla way" seems to be runtime detection, not compile-time detection

x86_32 and x86_64 translate __MMX__, __SSE__, __AVX__, etc. into MOZILLA_PRESUME_* which is later used to skip runtime checks e.g.,
https://searchfox.org/mozilla-central/rev/30b01f4f60db/mozglue/build/SSE.cpp#186-188

I'm deferring to :glandium for what he wants to do. Notice that bug 817042 never did get landed and I don't want to boil the ocean in this bug or we're never going to get support in for any OS.

I'm with Cameron in this, runtime detection is preferred in general on Power. So lets start with this and improve it incrementally. Also the x86 world is much more complex in the various SIMD acceleration features than the 3 flags for all Power/ppc cpus.

(In reply to Jan Beich from comment #11)
NetBSD does not currently support any of the CPUs providing this.
When this changes, it will be visible as sysctl (like currently machdep.altivec is for older CPUs)

:glandium, are you okay for reviewing this, or would someone else be better?

Flags: needinfo?(mh+mozilla)
Flags: needinfo?(mh+mozilla)
Attachment #9084366 - Attachment is obsolete: true

Thanks!

Keywords: checkin-needed

Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/252643ff91c5
basic support for Power ISA SIMD detection (v2). r=glandium

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 26 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Blocks: 1575802
Attachment #9084885 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.