Closed
Bug 684404
Opened 13 years ago
Closed 13 years ago
Fix ARM assembler VFP instructions
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bhackett1024, Assigned: mjrosenb)
References
Details
(Whiteboard: fixed-in-jaegermonkey)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
emitInst in the ARM assembler doesn't work correctly on floating point instructions, and can emit malformed instructions.
Reporter | ||
Updated•13 years ago
|
Assignee: general → mrosenberg
Assignee | ||
Comment 1•13 years ago
|
||
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.
Updated•13 years ago
|
OS: Mac OS X → All
Hardware: x86 → ARM
Reporter | ||
Comment 2•13 years ago
|
||
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.
Assignee | ||
Comment 3•13 years ago
|
||
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.
Attachment #558588 -
Attachment is obsolete: true
Reporter | ||
Comment 4•13 years ago
|
||
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)
Assignee | ||
Comment 5•13 years ago
|
||
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.
Assignee | ||
Comment 6•13 years ago
|
||
both previous patches rolled into one, with mq-goodness
Attachment #558856 -
Attachment is obsolete: true
Attachment #559270 -
Attachment is obsolete: true
Reporter | ||
Comment 7•13 years ago
|
||
Whiteboard: fixed-in-jaegermonkey
Reporter | ||
Comment 8•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•