1492302 - Implement CodeGeneratorARM64::visitDivI()

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Sean Stangl [:sstangl]]

Attachment: 0001-Implement-visitDivI.patch

Setting r? to jitbugs to see whether we pick up on that!

This implements the main division logic without the constant special-case paths. It mostly tracks the ARM implementation. There is one point at which x86/x64 have an OOL path that other architectures lack. I've left a TODO note so that it can be added at a later time, since that's more involved.

I'm hoping to keep these patches small to keep them easily-reviewable.

Comment 1

[Nicolas B. Pierron [:nbp]]

Comment on attachment 9010086 [details] [diff] [review]
0001-Implement-visitDivI.patch

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

Sounds good to me.

::: js/src/jit/arm64/CodeGenerator-arm64.cpp
@@ +439,5 @@
> +            masm.branch32(Assembler::NotEqual, rhs, Imm32(-1), &notOverflow);
> +            masm.wasmTrap(wasm::Trap::IntegerOverflow, mir->bytecodeOffset());
> +        } else if (mir->canTruncateOverflow()) {
> +            masm.branch32(Assembler::NotEqual, lhs, Imm32(INT32_MIN), &notOverflow);
> +            masm.branch32(Assembler::NotEqual, rhs, Imm32(-1), &notOverflow);

nit: These branches seems to be in common with all paths, factor them out.

@@ +448,5 @@
> +        } else {
> +            MOZ_ASSERT(mir->fallible());
> +            masm.branch32(Assembler::NotEqual, lhs, Imm32(INT32_MIN), &notOverflow);
> +            masm.cmp32(rhs, Imm32(-1));
> +            bailoutIf(Assembler::Equal, ins->snapshot());

nit: In which case this can become an unconditional bailout.

Comment 2

[Sean Stangl [:sstangl]]

Attachment: Patch to be checked in.

Patch to be checked in. I tried to do it myself, but the tree is closed, as it usually goes.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee5565ca40e2
Implement visitDivI(). r=nbp

Comment 4

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/ee5565ca40e2