487416 - TraceMonkey: Improve run-time detection of ARM processor features

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jacob Bramley [:jbramley]]

Attachment: Improve feature detection.

I want to be able to detect processor features for some other patches, but this is otherwise orthogonal so I have created a separate bug for this.

The attached patch expands the existing processor detection code to allow detection of more architecture variants and features such as Thumb and Thumb2.

Comment 1

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

Comment on attachment 371674 [details] [diff] [review]
Improve feature detection.

Looks good.  We may want to move the cpu detection code out of jstracer.cpp at some point, I guess, but no reason to do so now.

Comment 2

[Jacob Bramley [:jbramley]]

Yeah, I thought about that, but I opted to simply modify what was already there for now.

One thing to note: I haven't tested the WinCE case, as I don't have a WinCE platform. It looks Ok, but I can't be sure.

Comment 3

[Robert Sayre]

http://hg.mozilla.org/tracemonkey/rev/ef840521653f

Comment 4

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/ef840521653f

Comment 5

[Hiroyuki Ikezoe (:hiro)]

I got the following error. Am I missing something?

c:/cygwin/home/user/hg/mozilla-central/objdir-wm6-test/xulrunner/dist/sdk/bin/ar
m-wince-as.exe -o jswince.obj   /c/cygwin/home/user/hg/mozilla-central/js/src/js
wince.asm
Microsoft (R) ARM Macro Assembler Version 15.00.20720
Copyright (C) Microsoft Corporation.  All rights reserved.

c:\cygwin\home\user\hg\mozilla-central\js\src\jswince.asm(68) : error A0051: unk
nown opcode: DVD
    DVD 0xE6BFCF3C
c:\cygwin\home\user\hg\mozilla-central\js\src\jswince.asm(74) warning : A0241: I
nstruction blx not supported for -cpu "-arch 4t"
    blx js_arm_try_armv5_test
c:\cygwin\home\user\hg\mozilla-central\js\src\jswince.asm(74) : error A0034: und
efined symbol: js_arm_try_armv5_test
    blx js_arm_try_armv5_test
make[5]: *** [jswince.obj] Error 1
make[5]: Leaving directory `/c/cygwin/home/user/hg/mozilla-central/objdir-wm6-te
st/xulrunner/js/src'
make[4]: *** [libs_tier_js] Error 2
make[4]: Leaving directory `/c/cygwin/home/user/hg/mozilla-central/objdir-wm6-te
st/xulrunner'

Comment 6

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

Arg. I missed the typo in the wince asm -- that should be "DCD", not "DVD".  For the blx issue, that I don't know; I guess we should encode the BLX as a DCD as well just to be safe.

Comment 7

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

Pushed http://hg.mozilla.org/mozilla-central/rev/aa33c67004fb -- should fix, even though the assembler will still warn about blx.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Thank you, now I can go on building.

Comment 9

[Jacob Bramley [:jbramley]]

Apologies for that; I don't have a WinCE build environment to test with. Thanks Vlad for picking up the DCD.

Regarding BLX: BLX is an ARMv5 instruction, and WinCE — if I remember correctly — is only built for ARMv4, even though it typically runs on ARmv5+ devices. Thus, interworking (and BLX) will never be necessary on WinCE. BLX instructions will still work (with ARMv5), but may not be necessary.

If we encode the BLX using a DCD, we will avoid this warning and it will still work. The JIT will generate BLX only if it is supported.

---

Actually. looking at the encoding of BLX, it uses the 0xf condition code, which is defined as "unpredictable" on ARMv4, so BLX is probably not a good instruction to use to detect ARMv5 support. Having said that, we have decided (since I submitted this patch) that we don't need to support ARMv4, so it probably doesn't matter.

(The PLI instruction for ARMv7 support is Ok because that check isn't made until after the ARMv6 check is made, and ARMv6 handles 0xf in a predictable way.)

If only WinCE provided this information in the way that /proc does on Linux!

Comment 10

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

Yeah, the assembler still generates the right BLX opcode, so it's fine.

> If only WinCE provided this information in the way that /proc does on Linux!

On the flip side, you can test for any specific thing you want with WinCE, instead of being limited to just what's exposed by the currently running kernel :)

Comment 11

[Doug Turner (:dougt)]

this caused bug 492407 -- a startup crash on the HTC Touch Pro (Qualcomm MSM7201A 528 MHz (ARMv6, ARM1136EJ)).  Please backout or fix as soon as possible.

Comment 12

[Jacob Bramley [:jbramley]]

Attachment: Should fix the WinCE failure.

I've swapped the BLX for CLZ and so there should be no problem on WinCE. However, as before, I don't have a WinCE platform to test.

Comment 13

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

Comment on attachment 376877 [details] [diff] [review]
Should fix the WinCE failure.

This patch looks fine, but as the original patch already landed, this should just be a followup since it's impossible to land this as is.

Is the only change the armv5 BLX/CLZ detection implementation?

Comment 14

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

Pushed to both tm and mc:

http://hg.mozilla.org/tracemonkey/rev/3f263acde797
http://hg.mozilla.org/mozilla-central/rev/ee88749b4b6e

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Thank you, Jacob and Vladmir! Now crash has gone.