fix overflow issue in Num.isMultipleOf

This commit is contained in:
Folkert 2021-03-19 17:24:55 +01:00
parent c2da776ff4
commit b6d8b301d5
2 changed files with 58 additions and 26 deletions

View file

@ -4779,44 +4779,73 @@ fn build_int_binop<'a, 'ctx, 'env>(
NumLte => bd.build_int_compare(SLE, lhs, rhs, "int_lte").into(), NumLte => bd.build_int_compare(SLE, lhs, rhs, "int_lte").into(),
NumRemUnchecked => bd.build_int_signed_rem(lhs, rhs, "rem_int").into(), NumRemUnchecked => bd.build_int_signed_rem(lhs, rhs, "rem_int").into(),
NumIsMultipleOf => { NumIsMultipleOf => {
/* this builds the following construct // this builds the following construct
//
if rhs != 0 { // if (rhs == 0 || rhs == -1) {
let rem = lhs % rhs; // // lhs is a multiple of rhs iff
rem == 0 // //
} else { // // - rhs == -1
// rhs == 0 && lhs ==0 // // - both rhs and lhs are 0
lhs == 0 // //
} // // the -1 case is important for overflow reasons `isize::MIN % -1` crashes in rust
*/ // (rhs == -1) || (lhs == 0)
// } else {
// let rem = lhs % rhs;
// rem == 0
// }
//
// NOTE we'd like the branches to be swapped for better branch prediciton,
// but llvm normalizes to the above ordering in -O3
let zero = rhs.get_type().const_zero(); let zero = rhs.get_type().const_zero();
let condition_rhs = bd.build_int_compare(IntPredicate::NE, rhs, zero, "is_zero_rhs"); let neg_1 = rhs.get_type().const_int(-1i64 as u64, false);
let condition_lhs = bd.build_int_compare(IntPredicate::EQ, lhs, zero, "is_zero_lhs");
let current_block = bd.get_insert_block().unwrap(); //block that we are in right now; let special_block = env.context.append_basic_block(parent, "special_block");
let then_block = env.context.append_basic_block(parent, "then"); let default_block = env.context.append_basic_block(parent, "default_block");
let cont_block = env.context.append_basic_block(parent, "branchcont"); let cont_block = env.context.append_basic_block(parent, "branchcont");
bd.build_conditional_branch(condition_rhs, then_block, cont_block); bd.build_switch(
rhs,
default_block,
&[(zero, special_block), (neg_1, special_block)],
);
bd.position_at_end(then_block); let condition_rem = {
bd.position_at_end(default_block);
let rem = bd.build_int_signed_rem(lhs, rhs, "int_rem"); let rem = bd.build_int_signed_rem(lhs, rhs, "int_rem");
let condition_rem = bd.build_int_compare(IntPredicate::EQ, rem, zero, "is_zero_rem"); let result = bd.build_int_compare(IntPredicate::EQ, rem, zero, "is_zero_rem");
bd.build_unconditional_branch(cont_block);
result
};
let condition_special = {
bd.position_at_end(special_block);
let is_zero = bd.build_int_compare(IntPredicate::EQ, lhs, zero, "is_zero_lhs");
let is_neg_one =
bd.build_int_compare(IntPredicate::EQ, rhs, neg_1, "is_neg_one_rhs");
let result = bd.build_or(is_neg_one, is_zero, "cond");
bd.build_unconditional_branch(cont_block); bd.build_unconditional_branch(cont_block);
result
};
{
bd.position_at_end(cont_block); bd.position_at_end(cont_block);
let phi = bd.build_phi(env.context.bool_type(), "branch"); let phi = bd.build_phi(env.context.bool_type(), "branch");
phi.add_incoming(&[ phi.add_incoming(&[
(&condition_rem, then_block), (&condition_rem, default_block),
(&condition_lhs, current_block), (&condition_special, special_block),
]); ]);
phi.as_basic_value() phi.as_basic_value()
} }
}
NumDivUnchecked => bd.build_int_signed_div(lhs, rhs, "div_int").into(), NumDivUnchecked => bd.build_int_signed_div(lhs, rhs, "div_int").into(),
NumPowInt => call_bitcode_fn(env, &[lhs.into(), rhs.into()], &bitcode::NUM_POW_INT), NumPowInt => call_bitcode_fn(env, &[lhs.into(), rhs.into()], &bitcode::NUM_POW_INT),
NumBitwiseAnd => bd.build_and(lhs, rhs, "int_bitwise_and").into(), NumBitwiseAnd => bd.build_and(lhs, rhs, "int_bitwise_and").into(),

View file

@ -1389,5 +1389,8 @@ mod gen_num {
// false // false
assert_evals_to!("Num.isMultipleOf 5 2", false, bool); assert_evals_to!("Num.isMultipleOf 5 2", false, bool);
assert_evals_to!("Num.isMultipleOf 5 0", false, bool); assert_evals_to!("Num.isMultipleOf 5 0", false, bool);
// overflow
assert_evals_to!("Num.isMultipleOf -9223372036854775808 -1", true, bool);
} }
} }