468883 - Enable the ARMv6 pixman optimizations on Windows Mobile

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

MSVC doesn't support inline assembler for ARM so we'll need separate files that can be assembled with armasm. armasm does support the ARMv6 opcodes when using "-arch 6" flag.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

One of the things that we'll probably need for this is cpu type detection.

It looks like the emulators don't support the ARMv6 instructions. i.e. the give an illegal instruction exception when trying to execute them. So we'll hopefully be able to do runtime cpu detection using GetSystemInfo().

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Example program to print the cpu level

If someone could run this program and report the displayed message on a device with an ARMv6 or newer that would help in identifying whether GetSystemInfo is useful.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

on an HTC Touch Pro (Qualcomm MSM7201A 528 MHz, ARM1136EJ-S) I got "Result = 15 LEVEL:4" from your test program

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

fwiw, here is the full output from GetSystemInfo:
dwOemId: 5
wProcessorArchitecture: 5 (PROCESSOR_ARCHITECTURE_ARM)
wReserved: 0
dwPageSize: 4096
lpMinimumApplicationAddress: 65536
lpMaximumApplicationAddress: 2147483647
dwActiveProcessorMask: 1
dwNumberOfProcessors: 1
dwProcessorType: 2577  (PROCESSOR_STRONGARM)
dwAllocationGranularity: 65536
wProcessorLevel: 4
wProcessorRevision: 2

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

I thought IsProcessorFeaturePresent() might be useful, unfortunately it returns false for PR_ARM_V6.  Here is the full output:

PF_ARM_V4 supported
PF_ARM_V5 supported
PF_ARM_V6 not supported
PF_ARM_V7 not supported
PF_ARM_THUMB supported
PF_ARM_JAZELLE not supported
PF_ARM_DSP not supported
PF_ARM_MOVE_CP not supported
PF_ARM_VFP10 not supported
PF_ARM_MPU supported
PF_ARM_WRITE_BUFFER supported
PF_ARM_MBX not supported
PF_ARM_L2CACHE not supported
PF_ARM_PHYSICALLY_TAGGED_CACHE not supported
PF_ARM_VFP_SINGLE_PRECISION not supported
PF_ARM_VFP_DOUBLE_PRECISION not supported
PF_ARM_ITCM not supported
PF_ARM_DTCM not supported
PF_ARM_UNIFIED_CACHE not supported
PF_ARM_WRITE_BACK_CACHE supported
PF_ARM_CACHE_CAN_BE_LOCKED_DOWN supported
PF_ARM_L2CACHE_MEMORY_MAPPED not supported
PF_ARM_L2CACHE_COPROC not supported

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Attachment: try 2 at detecting armv6

Here's a new attempt that uses structured exception handling to catch EXCEPTION_ILLEGAL_INSTRUCTION and do the right thing.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

the pop up for that attachment read "CPU is ARMv6"

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Good, the easy part is done now. Next I'll try and put together a simple patch that gets something working.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

So based on VFP experiences, it looks like the kernel is probably built for ARMv5, even though the device itself is ARMv6.  I'd guess this is what IsProcessorFeaturePresent is returning.  It doesn't matter for the SIMD instructions, because those don't require any kernel support; but it does for VFP.

Getting this working probably means writing a native arm .asm file and avoiding inline assembly entirely; it might even be possible to use this cross-platform.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Attachment: Sketch of arm simd detection for pixman

Unbuilt and untested but the basic idea has been tested and it should work.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #9)
> Getting this working probably means writing a native arm .asm file and avoiding
> inline assembly entirely; it might even be possible to use this cross-platform.

Are the calling conventions the same on wince as on linux? If so, another option would be to just generate armasm files from the gcc produced binaries.

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

That's not a bad idea -- they are the same, yeah.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Attachment: First cut at generated asm

Here's the result from a first pass of generating the assembler to be fed into armasm. It assembles, I've no idea if it works.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: this seems to work

Jeff, this seems to work -- do we want to do any cleanup of this, or just stick a big ";;; WARNING - AUTO GENERATED" at the top?

Comment 15

[Jeff Muizelaar [:jrmuizel]]

Attachment: Cleanup of vlad's patch

This untested version uses an actual automatically generated file instead of one that's been hand munged. It also adds the runtime detection code.

Comment 16

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 362907 [details] [diff] [review]
Cleanup of vlad's patch

This has been working fine; let's go with it.

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: some smaller changes in Makefiles

Wait, one more patch.. some changes in the makefiles.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 363985 [details] [diff] [review]
some smaller changes in Makefiles

This breaks the linux build. I'll update the patch to fix it.

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Attachment: fix build on linux and a hint of cleanup

This should be better

Comment 20

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 363990 [details] [diff] [review]
fix build on linux and a hint of cleanup

Sounds good

Comment 21

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

http://hg.mozilla.org/mozilla-central/rev/357758846e59