Closed Bug 1518957 Opened 6 years ago Closed 6 years ago

Implement visitTrunc() and remove some unused ARM/ARM64 code.

Categories

(Core :: JavaScript Engine: JIT, enhancement, P2)

ARM64
Unspecified
enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: sstangl, Assigned: sstangl)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This patch implements visitTrunc() and visitTruncF(). Hopefully the logic is pretty straightforward.

The function emitRoundDouble() seems to be vestigial; on both ARM32 and ARM64 it exists but is completely unused. I removed it in this patch.

With this landed, the only real Ion codegen left to implement is the final vestiges of the specialized Div/Mod implementations. And whatever AsmJS wants. And maybe Wasm. And then watching it all break on real hardware :)

Passes ion/mathTrunc.js.

Attachment #9035482 - Flags: review?(nicolas.b.pierron)
Comment on attachment 9035482 [details] [diff] [review] 0001-Implement-ARM64-truncation-and-remove-unused-emitRou.patch Review of attachment 9035482 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/arm64/CodeGenerator-arm64.cpp @@ +1115,5 @@ > + masm.branch32(Assembler::Equal, output, Imm32(0), &zeroCase); > + > + // Bail on overflow cases. > + bailoutCmp32(Assembler::Equal, output, Imm32(INT_MAX), lir->snapshot()); > + bailoutCmp32(Assembler::Equal, output, Imm32(INT_MIN), lir->snapshot()); note: I fear that we might have a repeated bailout case here, as these are being monitored as Int32 and therefore not change the way this code is being compiled. As a follow-up bug, I suggest we add 2 branches to check if the double value is below INT_MAX+1 and above INT_MIN-1.
Attachment #9035482 - Flags: review?(nicolas.b.pierron) → review+
Keywords: checkin-needed
Blocks: 1519213

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7ad1e4500a9
Implement ARM64 truncation and remove unused emitRoundDouble(). r=nbp

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: