IonMonkey: MIPS: Refactor MacroAssembler::convertFloat32/DoubleToInt32

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: hev, Assigned: hev)

Tracking

unspecified
mozilla50
Other
Linux
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Remove branchs as soon as possible in generated code.
Attachment #8752540 - Flags: review?(hwjeastd07)
Attachment #8752540 - Flags: review?(arai.unmht)
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.
Attachment #8752540 - Flags: review?(arai.unmht) → review-
(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.
Attachment #8752540 - Attachment is obsolete: true
Attachment #8752540 - Flags: review?(hwjeastd07)
Attachment #8765346 - Flags: review?(arai.unmht)
Attachment #8765346 - Attachment is obsolete: true
Attachment #8765346 - Flags: review?(arai.unmht)
Comment on attachment 8765348 [details] [diff] [review]
0001-Bug-1272939-IonMonkey-MIPS-Refactor-MacroAssembler-c.patch

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

thanks :)
Attachment #8765348 - Flags: review?(arai.unmht) → review+
Pushed by r@hev.cc:
https://hg.mozilla.org/integration/mozilla-inbound/rev/984b53d372f7
IonMonkey: MIPS: Refactor MacroAssembler::convertFloat32/DoubleToInt32. r=arai
https://hg.mozilla.org/mozilla-central/rev/984b53d372f7
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.