585918 - JM: Implement branchTruncateDoubleToInt32 for ARM.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jacob Bramley [:jbramley]]

Attachment: Implement branchTruncateDoubleToInt32.

MacroAssemblerARM never actually had an implementation of branchTruncateDoubleToInt32. This causes about 12 trace tests to fail.

The attached patch adds an implementation that seems to work. Is it simply an implementation of ECMAScript's "ToInt32" conversion? (It looks like it, except that some cases are handled by a separate slow path.)

Comment 1

[David Mandelin [:dmandelin]]

Comment on attachment 464385 [details] [diff] [review]
Implement branchTruncateDoubleToInt32.

Yes, I believe this is intended to have the same semantics as the ECMA ToInt32 operation. This version looks good. Do you think the one in jsnum.h is faster?

We usually don't leave TODOs in SpiderMonkey code. If you think the jsnum.h solution is better, then you can just file a followup bug recommending that change.

Comment 2

[Jacob Bramley [:jbramley]]

(In reply to comment #1)
> Yes, I believe this is intended to have the same semantics as the ECMA ToInt32
> operation. This version looks good. Do you think the one in jsnum.h is faster?

It's different. The version in jsnum.h fully implements ToInt32, including NaN handling, overflows and all the corner cases. Here, we just fall back to a slow path.

VFP cannot fully implement ECMA's ToInt32 all by itself, so it needs help from integer code anyway. I experimented with various hybrids for jsnum.h and found that a pure soft-float implementation was best, but only because A8 doesn't do well at "VMOV <GPReg>, <FpReg>", and the source data was usually in memory anyway.

In this case, I think the VFP-based solution is best as the source is in VFP registers anyway (and so A8 will have to pay the transfer cost either way). I'll remove the TODO, but I'll open a bug to remind me to investigate further once ICs and TM integration are fixed etc. (This routine was a hot spot in jsnum.h so it's probably worth getting right here too.)

Comment 3

[Jacob Bramley [:jbramley]]

http://hg.mozilla.org/projects/jaegermonkey/rev/c5754250e813

Comment 4

[Jacob Bramley [:jbramley]]

My logic was broken :-(

Fixed here: http://hg.mozilla.org/projects/jaegermonkey/rev/5e81af26274c