Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: unchecked math operations in SSA #7011

Merged
merged 28 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
6331a91
Add some unchecked math operations
asterite Jan 9, 2025
3bb465a
Use unchecked add in `make_offset`
asterite Jan 9, 2025
41ee955
Use correct names when printing
asterite Jan 9, 2025
63af351
Parse unchecked ops
asterite Jan 9, 2025
a308767
Induction variable now uses unchecked add
asterite Jan 9, 2025
199a980
A few more cases where we need to check for unchecked ops
asterite Jan 9, 2025
f279e3d
A few more
asterite Jan 9, 2025
a2beb78
Use a boolean instead of another variant
asterite Jan 9, 2025
84de854
increment and decrement slice length
asterite Jan 9, 2025
20b323e
Merge branch 'master' into ab/unchecked-math
TomAFrench Jan 10, 2025
d230098
Merge branch 'master' into ab/unchecked-math
TomAFrench Jan 10, 2025
7dff466
Handle some TODOs
asterite Jan 10, 2025
626f0a2
Merge branch 'ab/unchecked-math' of github.com:noir-lang/noir into ab…
asterite Jan 10, 2025
dfad378
chore: address some todos
TomAFrench Jan 10, 2025
7840ced
Handle more TODOs
asterite Jan 10, 2025
c10520c
Merge branch 'ab/unchecked-math' of github.com:noir-lang/noir into ab…
asterite Jan 10, 2025
0df89c0
Handle one more TODO
asterite Jan 10, 2025
6f9d1c7
Fix tests
asterite Jan 10, 2025
f34fb51
Handle the final TODO
asterite Jan 10, 2025
e0ff78f
Don't emit unchecked operations for Fields, or when multiplying bools
asterite Jan 10, 2025
bacccc2
Add missing cast in remove_bit_shifts
asterite Jan 10, 2025
83f1fd8
Extract `self.check_binary_instruction`
asterite Jan 10, 2025
820c16c
Check binary instruction before simplification
asterite Jan 10, 2025
4683b0d
clippy
asterite Jan 10, 2025
cf1a9e7
Do cast in both cases
asterite Jan 10, 2025
ef30d7c
Only check and simplify binary instructions in `Binary::simplify`
asterite Jan 10, 2025
63cfd12
Fix comptime to_le_bits returning `[u8; N]` instead of `[u1; N]`
asterite Jan 10, 2025
9b42fd8
Allow casting u1 in comptime
asterite Jan 10, 2025
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
8 changes: 0 additions & 8 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,6 @@ impl FunctionBuilder {
operator: BinaryOp,
rhs: ValueId,
) -> ValueId {
let lhs_type = self.type_of_value(lhs);
let rhs_type = self.type_of_value(rhs);
if operator != BinaryOp::Shl && operator != BinaryOp::Shr {
assert_eq!(
lhs_type, rhs_type,
"ICE - Binary instruction operands must have the same type"
);
}
let instruction = Instruction::Binary(Binary { lhs, rhs, operator });
self.insert_instruction(instruction, None).first()
}
Expand Down
41 changes: 40 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
call_stack::{CallStack, CallStackHelper, CallStackId},
function::{FunctionId, RuntimeType},
instruction::{
Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction,
Binary, BinaryOp, Instruction, InstructionId, InstructionResultType, Intrinsic,
TerminatorInstruction,
},
map::DenseMap,
types::{NumericType, Type},
Expand Down Expand Up @@ -174,6 +175,44 @@
instruction_data: Instruction,
ctrl_typevars: Option<Vec<Type>>,
) -> InstructionId {
let instruction_data = if let Instruction::Binary(binary) = instruction_data {
let lhs = binary.lhs;
let rhs = binary.rhs;
let operator = binary.operator;
let lhs_type = self.type_of_value(lhs);
let rhs_type = self.type_of_value(rhs);
if operator != BinaryOp::Shl && operator != BinaryOp::Shr {
assert_eq!(
lhs_type, rhs_type,
"ICE - Binary instruction operands must have the same type"
);
}

let operator = if lhs_type.is_field() {
// Unchecked operations between fields or bools don't make sense, so we convert those to non-unchecked

Check warning on line 192 in compiler/noirc_evaluator/src/ssa/ir/dfg.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
// to reduce noise and confusion in the generated SSA.
match operator {
BinaryOp::Add { unchecked: true } => BinaryOp::Add { unchecked: false },
BinaryOp::Sub { unchecked: true } => BinaryOp::Sub { unchecked: false },
BinaryOp::Mul { unchecked: true } => BinaryOp::Mul { unchecked: false },
_ => operator,
}
} else if lhs_type.is_bool() {
// Unchecked mul between bools doesn't make sense, so we convert that to non-unchecked

Check warning on line 201 in compiler/noirc_evaluator/src/ssa/ir/dfg.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
if let BinaryOp::Mul { unchecked: true } = operator {
BinaryOp::Mul { unchecked: false }
} else {
operator
}
} else {
operator
};

Instruction::Binary(Binary { operator, ..binary })
} else {
instruction_data
};
asterite marked this conversation as resolved.
Show resolved Hide resolved

let id = self.instructions.insert(instruction_data);
self.make_instruction_results(id, ctrl_typevars);
id
Expand Down
10 changes: 10 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ impl Type {
matches!(self, Type::Numeric(NumericType::Unsigned { .. }))
}

/// Returns whether the `Type` represents the Field type.
pub(crate) fn is_field(&self) -> bool {
matches!(self, Type::Numeric(NumericType::NativeField))
}

/// Returns whether the `Type` is `u1`
pub(crate) fn is_bool(&self) -> bool {
matches!(self, Type::Numeric(NumericType::Unsigned { bit_size: 1 }))
}

/// Create a new signed integer type with the given amount of bits.
pub(crate) fn signed(bit_size: u32) -> Type {
Type::Numeric(NumericType::Signed { bit_size })
Expand Down
60 changes: 30 additions & 30 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -850,9 +850,9 @@ mod test {
enable_side_effects u1 1
v3 = cast v0 as Field
v4 = cast v1 as Field
v6 = unchecked_mul v3, Field 3
v8 = unchecked_mul v4, Field 4
v9 = unchecked_add v6, v8
v6 = mul v3, Field 3
v8 = mul v4, Field 4
v9 = add v6, v8
return v9
}
";
Expand Down Expand Up @@ -881,7 +881,7 @@ mod test {
acir(inline) fn main f0 {
b0(v0: u1, v1: u1):
enable_side_effects v0
v2 = unchecked_mul v1, v0
v2 = mul v1, v0
constrain v2 == v0
v3 = not v0
enable_side_effects u1 1
Expand Down Expand Up @@ -916,9 +916,9 @@ mod test {
v3 = not v0
v4 = cast v0 as Field
v5 = cast v3 as Field
v7 = unchecked_mul v4, Field 5
v8 = unchecked_mul v5, v2
v9 = unchecked_add v7, v8
v7 = mul v4, Field 5
v8 = mul v5, v2
v9 = add v7, v8
store v9 at v1
enable_side_effects u1 1
return
Expand Down Expand Up @@ -954,17 +954,17 @@ mod test {
v3 = not v0
v4 = cast v0 as Field
v5 = cast v3 as Field
v7 = unchecked_mul v4, Field 5
v8 = unchecked_mul v5, v2
v9 = unchecked_add v7, v8
v7 = mul v4, Field 5
v8 = mul v5, v2
v9 = add v7, v8
store v9 at v1
enable_side_effects v3
v10 = load v1 -> Field
v11 = cast v3 as Field
v12 = cast v0 as Field
v14 = unchecked_mul v11, Field 6
v15 = unchecked_mul v12, v10
v16 = unchecked_add v14, v15
v14 = mul v11, Field 6
v15 = mul v12, v10
v16 = add v14, v15
store v16 at v1
enable_side_effects u1 1
return
Expand Down Expand Up @@ -1066,32 +1066,32 @@ mod test {
v3 = not v0
v4 = cast v0 as Field
v5 = cast v3 as Field
v7 = unchecked_mul v4, Field 2
v8 = unchecked_add v7, v5
v9 = unchecked_mul v0, v1
v7 = mul v4, Field 2
v8 = add v7, v5
v9 = mul v0, v1
enable_side_effects v9
v10 = not v9
v11 = cast v9 as Field
v12 = cast v10 as Field
v14 = unchecked_mul v11, Field 5
v15 = unchecked_mul v12, v8
v16 = unchecked_add v14, v15
v14 = mul v11, Field 5
v15 = mul v12, v8
v16 = add v14, v15
v17 = not v1
v18 = unchecked_mul v0, v17
v18 = mul v0, v17
enable_side_effects v18
v19 = not v18
v20 = cast v18 as Field
v21 = cast v19 as Field
v23 = unchecked_mul v20, Field 6
v24 = unchecked_mul v21, v16
v25 = unchecked_add v23, v24
v23 = mul v20, Field 6
v24 = mul v21, v16
v25 = add v23, v24
enable_side_effects v0
enable_side_effects v3
v26 = cast v3 as Field
v27 = cast v0 as Field
v29 = unchecked_mul v26, Field 3
v30 = unchecked_mul v27, v25
v31 = unchecked_add v29, v30
v29 = mul v26, Field 3
v30 = mul v27, v25
v31 = add v29, v30
enable_side_effects u1 1
return v31
}";
Expand Down Expand Up @@ -1446,9 +1446,9 @@ mod test {
v2 = not v0
v3 = cast v0 as Field
v4 = cast v2 as Field
v6 = unchecked_mul v3, Field 2
v7 = unchecked_mul v4, v3
v8 = unchecked_add v6, v7
v6 = mul v3, Field 2
v7 = mul v4, v3
v8 = add v6, v7
enable_side_effects u1 1
return v8
}
Expand Down Expand Up @@ -1491,7 +1491,7 @@ mod test {
b0(v0: u1, v1: u1):
enable_side_effects v0
v2 = cast v0 as Field
v4 = unchecked_mul v2, Field 2
v4 = mul v2, Field 2
v5 = make_array [v4] : [Field; 1]
enable_side_effects u1 1
return v5
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ fn test_binary() {
let src = format!(
"
acir(inline) fn main f0 {{
b0(v0: Field, v1: Field):
b0(v0: u32, v1: u32):
v2 = {op} v0, v1
return
}}
Expand Down
Loading