486535 - TraceMonkey: ARM Thumb nanojit is not maintained and should be deleted.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jacob Bramley [:jbramley]]

This has been discussed (briefly) on #jsapi. The NativeThumb nanojit (for the ARM target) has not been maintained for some time and it doesn't seem to be possible to configure the build to include it. I have therefore made a tiny patch to remove it from Trace Monkey.

If Thumb(-2) support is required in the future, it would probably be better to port the existing NativeARM nanojit as it has optimizations and fixes which would probably be applicable to Thumb(-2) as well as ARM.

Comment 1

[Jacob Bramley [:jbramley]]

Attachment: Remove Thumb nanojit files and references.

Comment 2

[Jacob Bramley [:jbramley]]

Attachment: Remove Thumb nanojit files and references.

Fixed patch which actually deletes the relevant files.

Comment 3

[Nicholas Nethercote [inactive]]

I'd like to see this landed.  The patch is pretty simple and looks good to me, although I'm not an expert.

Comment 4

[Nicholas Nethercote [inactive]]

I'll reiterate that I'd like this to be landed -- when changing LIR and having to update the back-ends, it's annoying that the thumb back-end is there and must be ignored.

But I think there's more to be taken out.  If I grep for "thumb" I can see numerous things that should be removable in nanojit/avmplus.h, jstracer.cpp, and nanojit/NativeARM.cpp.

Comment 5

[Jacob Bramley [:jbramley]]

Nope, you can't remove them!

---

avmplus.h includes configuration flags (for AvmCore::config) which indicate whether or not Thumb and Thumb-2 are supported. Thumb support indicates that we need to generate interworking branches when calling native code* (as in bug 486639). Thumb-2 support is used to identify ARMv6T2 and above**, which support a few extra instructions that the JIT can use. Thus, even though we don't generate Thumb-2 code, there are still ARM instructions which were introduced with Thumb-2.

* This is interesting as we have since decided that we don't need to support ARMv4, and everything newer than that does support Thumb. Still, it's probably worth keeping the configuration flag as a sanity check.

** Actually the Linux detection code doesn't quite do this (due to auxv limitations) and it only detects Thumb-2 support for ARMv7 and above, but "if(thumb2)" will work with WinCE's platform detection code and it is more correct than "if(armv7)".

---

NativeARM.cpp contains an if(thumb2) to check whether or not it can use MOVW and MOVT. These were introduced with ARMv6T2 (as described above).

---

NativeARM.h contains two instances:
 * The first is a comment on the MOVZX8 macro, which is left over from the i386 JIT and does nothing on ARM. I removed this in bug 488845, so I wouldn't worry about it for now.
 * The second instance is a comment on the #endif at the end of the file, but this is already corrected by my patch for this bug.

---

Native.h contains two instances which are already removed by my patch.

---

Nativei386.h contains "Thumb" in a comment on line 452. I don't know x86 assembly at all, but I would guess that it shouldn't be there.

---

All other hits are in NativeThumb.cpp.

I think that explains all existing references to Thumb. Shout if something doesn't make sense!

Thanks,
Jacob

Comment 6

[Jacob Bramley [:jbramley]]

Attachment: Removes Thumb support. Re-synchronized with the tip.

Andreas, I've tagged you for review based on the e-mail discussion. Nicholas, if you want to steal the review, do so; it makes no difference to me :-)

Thanks,
Jacob

Comment 7

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/1b5584a4eba4

Comment 8

[Andreas Gal :gal]

Man hg sucks. Sorry for the repository mess.

http://hg.mozilla.org/tracemonkey/rev/28bfd4e38f84
http://hg.mozilla.org/tracemonkey/rev/7645eb3f210f
http://hg.mozilla.org/tracemonkey/rev/fe39d8a038db

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/7645eb3f210f