Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions JSTests/wasm/stress/armv7-i64-div-by-constant.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { instantiate } from "../wabt-wrapper.js"
import * as assert from "../assert.js"

// The i32 operand of the outer add is evaluated first, so it holds a register while the
// dividend is loaded. That pushes the dividend into a register pair overlapping the pair
// the C call passes it in, which is what makes the argument shuffle need a temporary.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely obvious what the issue with the argument shuffle needing a temporary is. Presumably it's because we place the value to be tested in the scratch registers in the needsZeroCheck case in emitModOrDiv (so we're assuming that the shuffle won't need the temporary), but this comment read pretty mysterious to me in this file :-)

let wat = `
(module
(func (export "divS") (param $x i64) (param $k i32) (result i32)
(i32.add (local.get $k) (i32.wrap_i64 (i64.div_s (local.get $x) (i64.const 1000)))))
(func (export "divU") (param $x i64) (param $k i32) (result i32)
(i32.add (local.get $k) (i32.wrap_i64 (i64.div_u (local.get $x) (i64.const 1000)))))
(func (export "remS") (param $x i64) (param $k i32) (result i32)
(i32.add (local.get $k) (i32.wrap_i64 (i64.rem_s (local.get $x) (i64.const 1000)))))
(func (export "remU") (param $x i64) (param $k i32) (result i32)
(i32.add (local.get $k) (i32.wrap_i64 (i64.rem_u (local.get $x) (i64.const 1000)))))
(func (export "divSLeft") (param $x i64) (param $k i32) (result i32)
(i32.add (local.get $k) (i32.wrap_i64 (i64.div_s (i64.const 1000) (local.get $x)))))
(func (export "remSLeft") (param $x i64) (param $k i32) (result i32)
(i32.add (local.get $k) (i32.wrap_i64 (i64.rem_s (i64.const 1000) (local.get $x)))))
(func (export "divS32") (param $x i32) (param $k i32) (result i32)
(i32.add (local.get $k) (i32.div_s (local.get $x) (i32.const 1000))))
)
`

async function test() {
const instance = await instantiate(wat, {}, { simd: true })
const { divS, divU, remS, remU, divSLeft, remSLeft, divS32 } = instance.exports

for (let i = 1; i <= 10000; ++i) {
let x = i * 7919
assert.eq(divS(BigInt(x), 5), 5 + (x / 1000 | 0))
assert.eq(divU(BigInt(x), 5), 5 + (x / 1000 | 0))
assert.eq(remS(BigInt(x), 5), 5 + x % 1000)
assert.eq(remU(BigInt(x), 5), 5 + x % 1000)
assert.eq(divSLeft(BigInt(x), 5), 5 + (1000 / x | 0))
assert.eq(remSLeft(BigInt(x), 5), 5 + 1000 % x)
assert.eq(divS32(x, 5), 5 + (x / 1000 | 0))
}

// A zero dividend with a non-zero constant divisor emits no zero check, so losing the
// divisor makes the runtime helper divide zero by zero and take SIGFPE.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I puzzled over "losing the divisor" here. IIUC this comment assumes the bug that's fixed by this commit. Perhaps "exercise the elided zero-check on the divisor"?

for (let i = 0; i < 10000; ++i) {
assert.eq(divS(0n, 5), 5)
assert.eq(divU(0n, 5), 5)
assert.eq(remS(0n, 5), 5)
assert.eq(remU(0n, 5), 5)
assert.eq(divS32(0, 5), 5)
}
}

await assert.asyncTest(test())
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/wasm/WasmBBQJIT32_64.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,8 @@ void BBQJIT::emitModOrDiv(Value& lhs, Location lhsLocation, Value& rhs, Location
}
}

auto lhsArg = Value::pinned(argType, lhsLocation);
auto rhsArg = Value::pinned(argType, rhsLocation);
auto lhsArg = lhs.isConst() ? lhs : Value::pinned(argType, lhsLocation);
auto rhsArg = rhs.isConst() ? rhs : Value::pinned(argType, rhsLocation);
consume(result);
emitCCall(modOrDiv, ArgumentList { lhsArg, rhsArg }, result);
}
Expand Down