Closed
Bug 1272939
Opened 8 years ago
Closed 8 years ago
IonMonkey: MIPS: Refactor MacroAssembler::convertFloat32/DoubleToInt32
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla50
| Tracking | Status | |
|---|---|---|
| firefox50 | --- | fixed |
People
(Reporter: hev, Assigned: hev)
Details
Attachments
(1 file, 2 obsolete files)
|
5.64 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
Remove branchs as soon as possible in generated code.
|
Assignee | |
Comment 1•8 years ago
|
||
Attachment #8752540 -
Flags: review?(hwjeastd07)
|
|
Assignee | |
Updated•8 years ago
|
Attachment #8752540 -
Flags: review?(arai.unmht)
|
||
Comment 2•8 years ago
|
||
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-
|
|
Assignee | |
Comment 3•8 years ago
|
||
(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.
|
|
Assignee | |
Comment 4•8 years ago
|
||
Attachment #8752540 -
Attachment is obsolete: true
Attachment #8752540 -
Flags: review?(hwjeastd07)
Attachment #8765346 -
Flags: review?(arai.unmht)
|
|
Assignee | |
Updated•8 years ago
|
Attachment #8765346 -
Attachment is obsolete: true
Attachment #8765346 -
Flags: review?(arai.unmht)
|
|
Assignee | |
Comment 5•8 years ago
|
||
Attachment #8765348 -
Flags: review?(arai.unmht)
|
|
||
Comment 6•8 years ago
|
||
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
|
||
Comment 8•8 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/984b53d372f7
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•