684404 - Fix ARM assembler VFP instructions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

emitInst in the ARM assembler doesn't work correctly on floating point instructions, and can emit malformed instructions.

Comment 1

[Marty Rosenberg [:mjrosenb]]

Attachment: Add in necessary wrappers to encod vfp instructions correctly.

Should fix the issue.  However, there are some functions (namely the last three in the file) that are only referred to by their pre-UAL names, and I don't know what the UAL names are, so I cannot actually verify that the encoding is correct with the spec. I've guessed, and testing seems to not show anything horribly awry.

Comment 2

[Brian Hackett [Laid off!]]

This patch seems to be broken for at least some combinations of registers when converting between doubles and int32s.  With this plus the patch in bug 669715, I looked at the first two failures I get with jit-tests --jitflags=mna.  I put simplified versions of these in my home directory on the ARM box, as failure-01.js and failure-02.js.  If you can figure out what's going on here that would be great.

failure-01.js

var x = 0;
var str = "a";
for (var i = 0; i < 10; ++i) {
    var y = str.charCodeAt(Infinity);
    print(y);
    x = y | 0;
    print(x);
}
assertEq(x, 0);

The second time around the loop (first time we are in the interpreter), we try to convert 'y' to an int32.  It is a NaN, but the code emitted by masm.branchTruncateDoubleToInt32 when called by Compiler::ensureInteger code passes the value through like it is an int32 and the 'y | 0' produces the wrong result.

failure-02.js

for (let i = 0; i != 30; i+=2) {
    print(i%4/2);
}

i%4 is a stub call which pushes an int32.  The DIV converts this to a double but always comes up with the value '0', so the result of the DIV is always zero rather than zero alternating with two.  Some disassembly:

=> 0x401809dc:	ldr	r8, [r10, #80]	; 0x50
=> 0x401809e0:	vmov	s0, r8
(gdb) p $r8
$10 = 2
=> 0x401809e4:	vcvt.f64.s32	d2, s2
(gdb) p $s0
$11 = 2.80259693e-45
(gdb) p $r8
$12 = 2
(gdb) p $s2
$13 = 0

It loads r8 from the stack and gets the right value, but d2 ends up as zero.

Comment 3

[Marty Rosenberg [:mjrosenb]]

Attachment: now with more fp-conversions

we were mis-handling the SN encoding scheme (0xf00 does not mask out bits 16-19, it masks out bits 8-13), and I believe the arguments for the two convert-dbl-to-int functions were mismatched.

Comment 4

[Brian Hackett [Laid off!]]

Next VFP bug...  I don't know if this is a separate issue from this patch or not, but when branchTruncateToInt32 is used to compile Math.floor in the below testcase, it clobbers the source FP register.  The source is still live, and is written out to stack and ends up producing the wrong result for one of the floor() calls (the 'res' string will have a NaN in it).


function dofloor(v)
{
  var res = "";
  for (var i = 0; i < 10; i++) {
    var q = Math.floor(v + i);
    res += q + ",";
  }
  assertEq(res, "2147483642,2147483643,2147483644,2147483645,2147483646,2147483647,2147483648,2147483649,2147483650,2147483651,");
}
dofloor(0x7ffffffd + .5);


Disassembly for part of the branchTruncateToInt32 call (I added an extra mov(d1, d3) in a futile attempt to avoid clobbering d1, but the result is the same with or without this mov).


=> 0x40166118:	vmov.f64	d3, d1
(gdb) p $d1   
$10 = 2147483647.5
(gdb) p $d3
$11 = 2147483646.5
(gdb) stepi
=> 0x4016611c:	vcvt.s32.f64	s3, d3
(gdb) p $d1
$12 = 2147483647.5
(gdb) p $d3
$13 = 2147483647.5
(gdb) p $s3
$14 = 27.9999981
(gdb) stepi
3: x/i $pc
=> 0x40166120:	vmov	r7, s3
(gdb) p $s3
$15 = nan(0x7fffff)
(gdb) p $d3
$16 = 2147483647.5
(gdb) p $d1
$17 = nan(0xfffffffe00000)

Comment 5

[Marty Rosenberg [:mjrosenb]]

Attachment: make the macro assembler use the correct registers for single values

the latest issue is that on arm, the single registers alias the double registers, but not in a 1-1 mapping; d1 aliases both s2 and s3.  In order to convert an integer to a double, we need to first copy the integer register into a single register, then convert the value in the single register into a double register.  We attempt to re-use the dest register as the single register that we copy the value into.  Unfortunately, we just re-use the number. In this case, we try to use d3 for the single value.  d3 is represented as "3", so we were in fact using s3, (which stomped on the value d1), rather than d6 or d7, which would have stomped on d3.

Comment 6

[Marty Rosenberg [:mjrosenb]]

Attachment: fix massive vfp breakage

both previous patches rolled into one, with mq-goodness

Comment 7

[Brian Hackett [Laid off!]]

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

Comment 8

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/mozilla-central/rev/c02868b913d5