Closed Bug 1518957 Opened 9 months ago Closed 8 months 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 2 open bugs)

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: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.