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)
Tracking
()
RESOLVED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: sstangl, Assigned: sstangl)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
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 1•6 years ago
|
||
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+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
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
Comment 3•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in
before you can comment on or make changes to this bug.
Description
•