Skip to content
Merged
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
105 changes: 104 additions & 1 deletion majit/majit-backend-wasm/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5626,7 +5626,7 @@ fn emit_guard_spill(

// ── Binary ops ──

#[derive(Clone, Copy)]
#[derive(Clone, Copy, Debug)]
enum BinOp {
I64Add,
I64Sub,
Expand Down Expand Up @@ -5795,6 +5795,53 @@ fn emit_umulhi_to_local(

/// Overflow binary op: stores the wrapping result in pos and the signed
/// overflow flag in the dedicated scratch local.
/// The overflow condition for `a + c` / `a - c` against a constant `c`, as
/// `(limit, greater_than)`: the operation overflows exactly when `a > limit`
/// (`greater_than`) or when `a < limit`. `None` means it cannot overflow.
///
/// The general form needs both operands and the result to compare sign bits.
/// Against a constant the same predicate is one comparison against a bound
/// folded here, which also drops the dependency on the result. Each bound is
/// taken from the opposite extreme, so none of them can itself overflow:
/// `MAX - c` only for `c > 0`, `MIN - c` only for `c < 0`, and so on.
fn ovf_const_bound(binop: BinOp, c: i64) -> Option<(i64, bool)> {
use std::cmp::Ordering;
match binop {
BinOp::I64Add => match c.cmp(&0) {
Ordering::Greater => Some((i64::MAX - c, true)),
Ordering::Less => Some((i64::MIN - c, false)),
Ordering::Equal => None,
},
BinOp::I64Sub => match c.cmp(&0) {
Ordering::Greater => Some((i64::MIN + c, false)),
Ordering::Less => Some((i64::MAX + c, true)),
Ordering::Equal => None,
},
_ => None,
}
}

/// The variable operand and constant operand of an add/sub whose overflow can
/// take the [`ovf_const_bound`] test. Addition is commutative, so either side
/// may supply the constant; for subtraction only the subtrahend does, since
/// `c - a` has a different bound shape and keeps the general form.
fn ovf_const_operand(
constants: &indexmap::IndexMap<u32, i64>,
op: &Op,
binop: BinOp,
) -> Option<(OpRef, i64)> {
let (a, b) = (op.arg(0).to_opref(), op.arg(1).to_opref());
match binop {
BinOp::I64Add if a.is_constant() && !b.is_constant() => {
Some((b, resolve_const_bits(constants, a)))
}
BinOp::I64Add | BinOp::I64Sub if !a.is_constant() && b.is_constant() => {
Some((a, resolve_const_bits(constants, b)))
}
_ => None,
}
}

fn emit_ovf_binop(
sink: &mut InstructionSink<'_>,
constants: &indexmap::IndexMap<u32, i64>,
Expand Down Expand Up @@ -5825,6 +5872,28 @@ fn emit_ovf_binop(
apply_binop(sink, binop);
sink.local_set(result_local);

if let Some((var, c)) = ovf_const_operand(constants, op, binop) {
match ovf_const_bound(binop, c) {
Some((limit, greater_than)) => {
emit_resolve(sink, constants, value_types, var);
sink.i64_const(limit);
if greater_than {
sink.i64_gt_s();
} else {
sink.i64_lt_s();
}
sink.i64_extend_i32_u();
}
// Adding or subtracting zero: the flag stays live so the paired
// guard still finds it, and folds against a constant zero.
None => {
sink.i64_const(0);
}
}
sink.local_set(ovf_flag_local);
return true;
}

match binop {
BinOp::I64Add => {
// ((a ^ result) & (b ^ result)) >>s 63
Expand Down Expand Up @@ -6157,4 +6226,38 @@ mod tests {
assert_eq!(frame.call_args_ofs, 416);
assert_eq!(frame.frame_bytes, 544);
}

/// The constant-operand bound must answer exactly what the wrapping
/// arithmetic does, including at the extremes where the bound itself is
/// closest to overflowing (`c` = `MIN` makes `MAX + c` and `MIN - c` the
/// interesting cases).
#[test]
fn ovf_const_bound_agrees_with_checked_arithmetic() {
let edges = [
i64::MIN,
i64::MIN + 1,
-3,
-1,
0,
1,
3,
i64::MAX - 1,
i64::MAX,
];
for &c in &edges {
for &a in &edges {
for (binop, expected) in [
(BinOp::I64Add, a.checked_add(c).is_none()),
(BinOp::I64Sub, a.checked_sub(c).is_none()),
] {
let got = match ovf_const_bound(binop, c) {
None => false,
Some((limit, true)) => a > limit,
Some((limit, false)) => a < limit,
};
assert_eq!(got, expected, "{binop:?}: a={a} c={c}");
}
}
}
}
}
103 changes: 103 additions & 0 deletions majit/majit-backend-wasm/tests/codegen_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,109 @@ fn execute_ovf_trace(opcode: OpCode, a: i64, b: i64) -> (i64, i64) {
execute_ovf_trace_with_guard(opcode, OpCode::GuardNoOverflow, a, b)
}

/// Same trace as [`execute_ovf_trace`], but the constant `c` is an operand
/// rather than a second input argument, so the overflow check takes the
/// folded-bound form instead of the sign-comparison one. `const_first` puts
/// it on the left, which only addition accepts.
fn execute_ovf_trace_const(opcode: OpCode, a: i64, c: i64, const_first: bool) -> (i64, i64) {
let inputargs = vec![InputArg::from_type(Type::Int, 0)];
let guard = Op::new(OpCode::GuardNoOverflow, &[]);
guard.setfailargs(smallvec![rb(OpRef::input_arg_int(0))]);
let finish = Op::new(OpCode::Finish, &[rb(OpRef::int_op(1))]);
finish.setfailargs(smallvec![rb(OpRef::int_op(1))]);
let args = if const_first {
[OpRef::const_int(c), OpRef::input_arg_int(0)]
} else {
[OpRef::input_arg_int(0), OpRef::const_int(c)]
};
let ops = vec![make_op(opcode, &args, OpRef::int_op(1)), guard, finish];
let (bytes, _) = build_module_default(&inputargs, &ops, &indexmap::IndexMap::new());

let engine = Engine::default();
let module = Module::new(&engine, &bytes).expect("generated trace should compile");
let mut store = Store::new(&engine, ());
let memory =
Memory::new(&mut store, MemoryType::new(1, None)).expect("test memory should allocate");
memory
.write(
&mut store,
codegen::FRAME_SLOT_BASE as usize,
&a.to_le_bytes(),
)
.unwrap();
let mut linker = Linker::new(&engine);
linker.define("env", "memory", memory).unwrap();
let instance = linker
.instantiate_and_start(&mut store, &module)
.expect("generated trace should instantiate");
instance
.get_typed_func::<i32, i32>(&store, "trace")
.unwrap()
.call(&mut store, 0)
.expect("generated trace should execute");

let mut fail_index = [0; 8];
let mut result = [0; 8];
memory.read(&store, 0, &mut fail_index).unwrap();
memory
.read(&store, codegen::FRAME_SLOT_BASE as usize, &mut result)
.unwrap();
(i64::from_le_bytes(fail_index), i64::from_le_bytes(result))
}

/// A constant operand takes the folded-bound overflow check, so it needs the
/// same verdicts as the general form at the extremes — including the two
/// bounds that sit closest to overflowing themselves, `a - i64::MIN` and
/// `a + i64::MIN`.
#[test]
fn test_ovf_against_a_constant_matches_the_general_form() {
// (opcode, a, c, overflows)
let cases = [
(OpCode::IntAddOvf, 10, 20, false),
(OpCode::IntAddOvf, 5, 0, false),
(OpCode::IntAddOvf, i64::MAX, 1, true),
(OpCode::IntAddOvf, i64::MAX - 1, 1, false),
(OpCode::IntAddOvf, 10, -20, false),
(OpCode::IntAddOvf, i64::MIN, -1, true),
(OpCode::IntAddOvf, -1, i64::MIN, true),
(OpCode::IntAddOvf, 0, i64::MIN, false),
(OpCode::IntSubOvf, 100, 58, false),
(OpCode::IntSubOvf, 5, 0, false),
(OpCode::IntSubOvf, i64::MIN, 1, true),
(OpCode::IntSubOvf, i64::MIN + 1, 1, false),
(OpCode::IntSubOvf, 10, -5, false),
(OpCode::IntSubOvf, i64::MAX, -1, true),
(OpCode::IntSubOvf, 0, i64::MIN, true),
(OpCode::IntSubOvf, -1, i64::MIN, false),
];
for (opcode, a, c, overflows) in cases {
let (fail_index, result) = execute_ovf_trace_const(opcode, a, c, false);
if overflows {
assert_eq!(fail_index, 0, "{opcode:?}: {a} op {c} should guard-exit");
} else {
let expected = match opcode {
OpCode::IntAddOvf => a.wrapping_add(c),
_ => a.wrapping_sub(c),
};
assert_eq!(
(fail_index, result),
(1, expected),
"{opcode:?}: {a} op {c}"
);
}
}

// Addition is commutative, so the constant is also accepted on the left.
assert_eq!(
execute_ovf_trace_const(OpCode::IntAddOvf, 10, 20, true),
(1, 30)
);
assert_eq!(
execute_ovf_trace_const(OpCode::IntAddOvf, i64::MAX, 1, true).0,
0
);
}

#[test]
fn test_int_add_ovf_guards_overflow() {
for (a, b, expected) in [(10, 20, 30), (i64::MIN, 1, i64::MIN + 1)] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=637
guard_failures=636
internal_compile_panics=0
loops_aborted=0
loops_compiled=3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=806
guard_failures=805
internal_compile_panics=0
loops_aborted=0
loops_compiled=2
Expand Down
Loading
Loading