Skip to content

Commit 2187209

Browse files
committed
fix: remove saturating behaviour of signed bitshifts in brillig
1 parent e121b9b commit 2187209

File tree

1 file changed

+25
-29
lines changed

1 file changed

+25
-29
lines changed

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,7 +1701,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
17011701

17021702
self.brillig_context.codegen_branch(left_is_negative.address, |ctx, is_negative| {
17031703
if is_negative {
1704-
// If right value is greater than the left bit size, return -1
1704+
// Assert right value is less than the left bit size.
1705+
// We only need to assert this here as the brillig shr opcode implies this constraint.
17051706
let rhs_does_not_overflow = SingleAddrVariable::new(ctx.allocate_register(), 1);
17061707
let lhs_bit_size =
17071708
ctx.make_constant_instruction(left.bit_size.into(), right.bit_size);
@@ -1711,35 +1712,30 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
17111712
rhs_does_not_overflow,
17121713
BrilligBinaryOp::LessThan,
17131714
);
1715+
ctx.codegen_constrain(rhs_does_not_overflow, None);
17141716

1715-
ctx.codegen_branch(rhs_does_not_overflow.address, |ctx, no_overflow| {
1716-
if no_overflow {
1717-
let one = ctx.make_constant_instruction(1_u128.into(), left.bit_size);
1718-
1719-
// computes 2^right
1720-
let two = ctx.make_constant_instruction(2_u128.into(), left.bit_size);
1721-
let two_pow = ctx.make_constant_instruction(1_u128.into(), left.bit_size);
1722-
let right_u32 = SingleAddrVariable::new(ctx.allocate_register(), 32);
1723-
ctx.cast(right_u32, right);
1724-
let pow_body = |ctx: &mut BrilligContext<_, _>, _: SingleAddrVariable| {
1725-
ctx.binary_instruction(two_pow, two, two_pow, BrilligBinaryOp::Mul);
1726-
};
1727-
ctx.codegen_for_loop(None, right_u32.address, None, pow_body);
1728-
1729-
// Right shift using division on 1-complement
1730-
ctx.binary_instruction(left, one, result, BrilligBinaryOp::Add);
1731-
ctx.convert_signed_division(result, two_pow, result);
1732-
ctx.binary_instruction(result, one, result, BrilligBinaryOp::Sub);
1733-
1734-
// Clean-up
1735-
ctx.deallocate_single_addr(one);
1736-
ctx.deallocate_single_addr(two);
1737-
ctx.deallocate_single_addr(two_pow);
1738-
ctx.deallocate_single_addr(right_u32);
1739-
} else {
1740-
ctx.const_instruction(result, ((1_u128 << left.bit_size) - 1).into());
1741-
}
1742-
});
1717+
let one = ctx.make_constant_instruction(1_u128.into(), left.bit_size);
1718+
1719+
// computes 2^right
1720+
let two = ctx.make_constant_instruction(2_u128.into(), left.bit_size);
1721+
let two_pow = ctx.make_constant_instruction(1_u128.into(), left.bit_size);
1722+
let right_u32 = SingleAddrVariable::new(ctx.allocate_register(), 32);
1723+
ctx.cast(right_u32, right);
1724+
let pow_body = |ctx: &mut BrilligContext<_, _>, _: SingleAddrVariable| {
1725+
ctx.binary_instruction(two_pow, two, two_pow, BrilligBinaryOp::Mul);
1726+
};
1727+
ctx.codegen_for_loop(None, right_u32.address, None, pow_body);
1728+
1729+
// Right shift using division on 1-complement
1730+
ctx.binary_instruction(left, one, result, BrilligBinaryOp::Add);
1731+
ctx.convert_signed_division(result, two_pow, result);
1732+
ctx.binary_instruction(result, one, result, BrilligBinaryOp::Sub);
1733+
1734+
// Clean-up
1735+
ctx.deallocate_single_addr(one);
1736+
ctx.deallocate_single_addr(two);
1737+
ctx.deallocate_single_addr(two_pow);
1738+
ctx.deallocate_single_addr(right_u32);
17431739

17441740
ctx.deallocate_single_addr(rhs_does_not_overflow);
17451741
} else {

0 commit comments

Comments
 (0)