Skip to content

Commit d86ff1a

Browse files
authored
feat: allow bitshifts to be represented in SSA for brillig (#4301)
# Description ## Problem\* Resolves #4276 ## Summary\* Quick and dirty implementation of replacing bitshifts in SSA once function inlining has occurred. Not a fan of how we are juggling the instructions but will need to think about how to do this cleanly. cc @sirasistant ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
1 parent 1786d8a commit d86ff1a

9 files changed

Lines changed: 330 additions & 134 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,8 @@ pub(crate) fn convert_ssa_binary_op_to_brillig_binary_op(
14491449
BinaryOp::And => BinaryIntOp::And,
14501450
BinaryOp::Or => BinaryIntOp::Or,
14511451
BinaryOp::Xor => BinaryIntOp::Xor,
1452+
BinaryOp::Shl => BinaryIntOp::Shl,
1453+
BinaryOp::Shr => BinaryIntOp::Shr,
14521454
};
14531455

14541456
BrilligBinaryOp::Integer { op: operation, bit_size }

compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,8 @@ impl DebugToString for BinaryIntOp {
7474
BinaryIntOp::And => "&&".into(),
7575
BinaryIntOp::Or => "||".into(),
7676
BinaryIntOp::Xor => "^".into(),
77-
BinaryIntOp::Shl | BinaryIntOp::Shr => {
78-
unreachable!("bit shift should have been replaced")
79-
}
77+
BinaryIntOp::Shl => "<<".into(),
78+
BinaryIntOp::Shr => ">>".into(),
8079
}
8180
}
8281
}

compiler/noirc_evaluator/src/ssa.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ pub(crate) fn optimize_into_acir(
6060
// and this pass is missed, slice merging will fail inside of flattening.
6161
.run_pass(Ssa::mem2reg, "After Mem2Reg:")
6262
.run_pass(Ssa::flatten_cfg, "After Flattening:")
63+
.run_pass(Ssa::remove_bit_shifts, "After Removing Bit Shifts:")
6364
// Run mem2reg once more with the flattened CFG to catch any remaining loads/stores
6465
.run_pass(Ssa::mem2reg, "After Mem2Reg:")
6566
.run_pass(Ssa::fold_constants, "After Constant Folding:")

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,9 @@ impl Context {
14831483
bit_count,
14841484
self.current_side_effects_enabled_var,
14851485
),
1486+
BinaryOp::Shl | BinaryOp::Shr => unreachable!(
1487+
"ICE - bit shift operators do not exist in ACIR and should have been replaced"
1488+
),
14861489
}
14871490
}
14881491

compiler/noirc_evaluator/src/ssa/function_builder/mod.rs

Lines changed: 1 addition & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ use super::{
1818
basic_block::BasicBlock,
1919
dfg::{CallStack, InsertInstructionResult},
2020
function::RuntimeType,
21-
instruction::{ConstrainError, Endian, InstructionId, Intrinsic},
22-
types::NumericType,
21+
instruction::{ConstrainError, InstructionId, Intrinsic},
2322
},
2423
ssa_gen::Ssa,
2524
};
@@ -279,115 +278,6 @@ impl FunctionBuilder {
279278
self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)).results()
280279
}
281280

282-
/// Insert ssa instructions which computes lhs << rhs by doing lhs*2^rhs
283-
/// and truncate the result to bit_size
284-
pub(crate) fn insert_wrapping_shift_left(
285-
&mut self,
286-
lhs: ValueId,
287-
rhs: ValueId,
288-
bit_size: u32,
289-
) -> ValueId {
290-
let base = self.field_constant(FieldElement::from(2_u128));
291-
let typ = self.current_function.dfg.type_of_value(lhs);
292-
let (max_bit, pow) =
293-
if let Some(rhs_constant) = self.current_function.dfg.get_numeric_constant(rhs) {
294-
// Happy case is that we know precisely by how many bits the the integer will
295-
// increase: lhs_bit_size + rhs
296-
let bit_shift_size = rhs_constant.to_u128() as u32;
297-
298-
let (rhs_bit_size_pow_2, overflows) = 2_u128.overflowing_pow(bit_shift_size);
299-
if overflows {
300-
assert!(bit_size < 128, "ICE - shift left with big integers are not supported");
301-
if bit_size < 128 {
302-
let zero = self.numeric_constant(FieldElement::zero(), typ);
303-
return InsertInstructionResult::SimplifiedTo(zero).first();
304-
}
305-
}
306-
let pow = self.numeric_constant(FieldElement::from(rhs_bit_size_pow_2), typ);
307-
308-
let max_lhs_bits = self.current_function.dfg.get_value_max_num_bits(lhs);
309-
310-
(max_lhs_bits + bit_shift_size, pow)
311-
} else {
312-
// we use a predicate to nullify the result in case of overflow
313-
let bit_size_var =
314-
self.numeric_constant(FieldElement::from(bit_size as u128), typ.clone());
315-
let overflow = self.insert_binary(rhs, BinaryOp::Lt, bit_size_var);
316-
let predicate = self.insert_cast(overflow, typ.clone());
317-
// we can safely cast to unsigned because overflow_checks prevent bit-shift with a negative value
318-
let rhs_unsigned = self.insert_cast(rhs, Type::unsigned(bit_size));
319-
let pow = self.pow(base, rhs_unsigned);
320-
let pow = self.insert_cast(pow, typ);
321-
(FieldElement::max_num_bits(), self.insert_binary(predicate, BinaryOp::Mul, pow))
322-
};
323-
324-
if max_bit <= bit_size {
325-
self.insert_binary(lhs, BinaryOp::Mul, pow)
326-
} else {
327-
let result = self.insert_binary(lhs, BinaryOp::Mul, pow);
328-
self.insert_truncate(result, bit_size, max_bit)
329-
}
330-
}
331-
332-
/// Insert ssa instructions which computes lhs >> rhs by doing lhs/2^rhs
333-
pub(crate) fn insert_shift_right(
334-
&mut self,
335-
lhs: ValueId,
336-
rhs: ValueId,
337-
bit_size: u32,
338-
) -> ValueId {
339-
let lhs_typ = self.type_of_value(lhs);
340-
let base = self.field_constant(FieldElement::from(2_u128));
341-
// we can safely cast to unsigned because overflow_checks prevent bit-shift with a negative value
342-
let rhs_unsigned = self.insert_cast(rhs, Type::unsigned(bit_size));
343-
let pow = self.pow(base, rhs_unsigned);
344-
// We need at least one more bit for the case where rhs == bit_size
345-
let div_type = Type::unsigned(bit_size + 1);
346-
let casted_lhs = self.insert_cast(lhs, div_type.clone());
347-
let casted_pow = self.insert_cast(pow, div_type);
348-
let div_result = self.insert_binary(casted_lhs, BinaryOp::Div, casted_pow);
349-
// We have to cast back to the original type
350-
self.insert_cast(div_result, lhs_typ)
351-
}
352-
353-
/// Computes lhs^rhs via square&multiply, using the bits decomposition of rhs
354-
/// Pseudo-code of the computation:
355-
/// let mut r = 1;
356-
/// let rhs_bits = to_bits(rhs);
357-
/// for i in 1 .. bit_size + 1 {
358-
/// let r_squared = r * r;
359-
/// let b = rhs_bits[bit_size - i];
360-
/// r = (r_squared * lhs * b) + (1 - b) * r_squared;
361-
/// }
362-
pub(crate) fn pow(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId {
363-
let typ = self.current_function.dfg.type_of_value(rhs);
364-
if let Type::Numeric(NumericType::Unsigned { bit_size }) = typ {
365-
let to_bits = self.import_intrinsic_id(Intrinsic::ToBits(Endian::Little));
366-
let length = self.field_constant(FieldElement::from(bit_size as i128));
367-
let result_types =
368-
vec![Type::field(), Type::Array(Rc::new(vec![Type::bool()]), bit_size as usize)];
369-
let rhs_bits = self.insert_call(to_bits, vec![rhs, length], result_types);
370-
let rhs_bits = rhs_bits[1];
371-
let one = self.field_constant(FieldElement::one());
372-
let mut r = one;
373-
for i in 1..bit_size + 1 {
374-
let r_squared = self.insert_binary(r, BinaryOp::Mul, r);
375-
let a = self.insert_binary(r_squared, BinaryOp::Mul, lhs);
376-
let idx = self.field_constant(FieldElement::from((bit_size - i) as i128));
377-
let b = self.insert_array_get(rhs_bits, idx, Type::bool());
378-
let not_b = self.insert_not(b);
379-
let b = self.insert_cast(b, Type::field());
380-
let not_b = self.insert_cast(not_b, Type::field());
381-
let r1 = self.insert_binary(a, BinaryOp::Mul, b);
382-
let r2 = self.insert_binary(r_squared, BinaryOp::Mul, not_b);
383-
r = self.insert_binary(r1, BinaryOp::Add, r2);
384-
}
385-
r
386-
} else {
387-
unreachable!("Value must be unsigned in power operation");
388-
}
389-
}
390-
391281
/// Insert an instruction to extract an element from an array
392282
pub(crate) fn insert_array_get(
393283
&mut self,

compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ pub(crate) enum BinaryOp {
3838
Or,
3939
/// Bitwise xor (^)
4040
Xor,
41+
/// Bitshift left (<<)
42+
Shl,
43+
/// Bitshift right (>>)
44+
Shr,
4145
}
4246

4347
impl std::fmt::Display for BinaryOp {
@@ -53,6 +57,8 @@ impl std::fmt::Display for BinaryOp {
5357
BinaryOp::And => write!(f, "and"),
5458
BinaryOp::Or => write!(f, "or"),
5559
BinaryOp::Xor => write!(f, "xor"),
60+
BinaryOp::Shl => write!(f, "shl"),
61+
BinaryOp::Shr => write!(f, "shr"),
5662
}
5763
}
5864
}
@@ -215,7 +221,27 @@ impl Binary {
215221
return SimplifyResult::SimplifiedTo(zero);
216222
}
217223
}
218-
}
224+
BinaryOp::Shl => return SimplifyResult::None,
225+
BinaryOp::Shr => {
226+
// Bit shifts by constants can be treated as divisions.
227+
if let Some(rhs_const) = rhs {
228+
if rhs_const >= FieldElement::from(operand_type.bit_size() as u128) {
229+
// Shifting by the full width of the operand type, any `lhs` goes to zero.
230+
let zero = dfg.make_constant(FieldElement::zero(), operand_type);
231+
return SimplifyResult::SimplifiedTo(zero);
232+
}
233+
234+
// `two_pow_rhs` is limited to be at most `2 ^ {operand_bitsize - 1}` so it fits in `operand_type`.
235+
let two_pow_rhs = FieldElement::from(2u128).pow(&rhs_const);
236+
let two_pow_rhs = dfg.make_constant(two_pow_rhs, operand_type);
237+
return SimplifyResult::SimplifiedToInstruction(Instruction::binary(
238+
BinaryOp::Div,
239+
self.lhs,
240+
two_pow_rhs,
241+
));
242+
}
243+
}
244+
};
219245
SimplifyResult::None
220246
}
221247
}
@@ -314,6 +340,8 @@ impl BinaryOp {
314340
BinaryOp::And => None,
315341
BinaryOp::Or => None,
316342
BinaryOp::Xor => None,
343+
BinaryOp::Shl => None,
344+
BinaryOp::Shr => None,
317345
}
318346
}
319347

@@ -329,6 +357,8 @@ impl BinaryOp {
329357
BinaryOp::Xor => |x, y| Some(x ^ y),
330358
BinaryOp::Eq => |x, y| Some((x == y) as u128),
331359
BinaryOp::Lt => |x, y| Some((x < y) as u128),
360+
BinaryOp::Shl => |x, y| Some(x << y),
361+
BinaryOp::Shr => |x, y| Some(x >> y),
332362
}
333363
}
334364

@@ -344,6 +374,8 @@ impl BinaryOp {
344374
BinaryOp::Xor => |x, y| Some(x ^ y),
345375
BinaryOp::Eq => |x, y| Some((x == y) as i128),
346376
BinaryOp::Lt => |x, y| Some((x < y) as i128),
377+
BinaryOp::Shl => |x, y| Some(x << y),
378+
BinaryOp::Shr => |x, y| Some(x >> y),
347379
}
348380
}
349381
}

compiler/noirc_evaluator/src/ssa/opt/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@ mod die;
1212
pub(crate) mod flatten_cfg;
1313
mod inlining;
1414
mod mem2reg;
15+
mod remove_bit_shifts;
1516
mod simplify_cfg;
1617
mod unrolling;

0 commit comments

Comments
 (0)