1272939 - IonMonkey: MIPS: Refactor MacroAssembler::convertFloat32/DoubleToInt32

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[hev [:hev]]

Remove branchs as soon as possible in generated code.

Comment 1

[hev [:hev]]

Attachment: 0001-Bug-1272939-IonMonkey-MIPS-Refactor-MacroAssembler-c.patch

Comment 2

[Tooru Fujisawa [:arai]]

Comment on attachment 8752540 [details] [diff] [review]
0001-Bug-1272939-IonMonkey-MIPS-Refactor-MacroAssembler-c.patch

Review of attachment 8752540 [details] [diff] [review]:
-----------------------------------------------------------------

Looks almost good, but there are one place that introduces different behavior.
can you fix it?

::: js/src/jit/mips32/MacroAssembler-mips32.cpp
@@ +123,5 @@
> +    if (negativeZeroCheck) {
> +        moveFromDoubleHi(src, dest);
> +        moveFromDoubleLo(src, ScratchRegister);
> +        ma_or(dest, ScratchRegister);
> +        ma_b(dest, Imm32(INT32_MIN), fail, Assembler::Equal);

This jumps to fail also if src is 0x8000000080000000, that is not negative zero.

Comment 3

[hev [:hev]]

(In reply to Tooru Fujisawa [:arai] from comment #2)
> Comment on attachment 8752540 [details] [diff] [review]
> 0001-Bug-1272939-IonMonkey-MIPS-Refactor-MacroAssembler-c.patch
> 
> Review of attachment 8752540 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks almost good, but there are one place that introduces different
> behavior.
> can you fix it?
> 
> ::: js/src/jit/mips32/MacroAssembler-mips32.cpp
> @@ +123,5 @@
> > +    if (negativeZeroCheck) {
> > +        moveFromDoubleHi(src, dest);
> > +        moveFromDoubleLo(src, ScratchRegister);
> > +        ma_or(dest, ScratchRegister);
> > +        ma_b(dest, Imm32(INT32_MIN), fail, Assembler::Equal);
> 
> This jumps to fail also if src is 0x8000000080000000, that is not negative
> zero.

oops, thank you.

Comment 4

[hev [:hev]]

Attachment: 0001-Bug-1272939-IonMonkey-MIPS-Refactor-MacroAssembler-c.patch

Comment 5

[hev [:hev]]

Attachment: 0001-Bug-1272939-IonMonkey-MIPS-Refactor-MacroAssembler-c.patch

Comment 6

[Tooru Fujisawa [:arai]]

Comment on attachment 8765348 [details] [diff] [review]
0001-Bug-1272939-IonMonkey-MIPS-Refactor-MacroAssembler-c.patch

Review of attachment 8765348 [details] [diff] [review]:
-----------------------------------------------------------------

thanks :)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/984b53d372f7
IonMonkey: MIPS: Refactor MacroAssembler::convertFloat32/DoubleToInt32. r=arai

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/984b53d372f7