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

pilopt: equal-constrained witness columns removal #2086

Closed
wants to merge 57 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
c9606ed
Opt: equal-constrained witness columns removal
gzanitti Nov 13, 2024
567e7f1
avoid removing columns when next is true
gzanitti Nov 13, 2024
f1a1f6e
some reparse::* failing
gzanitti Nov 13, 2024
761dc48
remove removed
gzanitti Nov 14, 2024
0df29b4
code simplification
gzanitti Nov 14, 2024
e0f7b18
N::Y = N::Y not being caught
gzanitti Nov 14, 2024
b4e6bd5
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Nov 14, 2024
86f9f2c
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Nov 14, 2024
fde0d6b
removed A = A case from identities
gzanitti Nov 14, 2024
684b979
preserve order
gzanitti Nov 14, 2024
523a0d8
remove after simplification
gzanitti Nov 14, 2024
6efa22d
always left
gzanitti Nov 14, 2024
99cb450
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Nov 15, 2024
b02522a
Code improvements & test fix
gzanitti Nov 15, 2024
278ed35
remove_unreferenced_definitions before remove_trivial_identities to l…
gzanitti Nov 15, 2024
b1c56e4
- set_hint
gzanitti Nov 15, 2024
ede7992
checking and updating defs
gzanitti Nov 19, 2024
3c378a3
Docs, msgs, unused imports, etc
gzanitti Nov 19, 2024
a3dd4be
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Nov 19, 2024
d8cbf75
set_hint?
gzanitti Nov 19, 2024
19aca28
avoid get values from *_const in RISCV when pilopt removed them
gzanitti Nov 20, 2024
a4f6a4e
contains_key
gzanitti Nov 20, 2024
906a75b
fix output
gzanitti Nov 20, 2024
af1f679
change set_hint() test to avoid opt
gzanitti Nov 20, 2024
be6f87b
minor fix
gzanitti Nov 20, 2024
a8f0864
empty spaces & minor details
gzanitti Nov 20, 2024
83a8b88
namespace & try_get_col
gzanitti Nov 20, 2024
2622366
avoid double call
gzanitti Nov 20, 2024
239d573
more tests
gzanitti Nov 21, 2024
dfc9424
avoid array elem
gzanitti Nov 21, 2024
dde999a
names
gzanitti Nov 21, 2024
b4f485e
Function simplification, new doc, etc
gzanitti Nov 25, 2024
c53022c
Minor issue after merge main fixed
gzanitti Nov 29, 2024
b9cbcbe
only update definitions
gzanitti Nov 29, 2024
d7e6875
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Nov 29, 2024
39dba03
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Nov 29, 2024
32e74ef
back
gzanitti Nov 29, 2024
ba335e8
missed println
gzanitti Nov 29, 2024
92b21bf
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Dec 2, 2024
a86bd85
test estark
gzanitti Dec 2, 2024
5d070be
lint
gzanitti Dec 2, 2024
ecde331
removed from lookups
gzanitti Dec 2, 2024
8019fc4
cols in lookup & better code
gzanitti Dec 2, 2024
a213672
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Dec 2, 2024
0b3e774
toopt, removed
gzanitti Dec 2, 2024
79d3d80
build
gzanitti Dec 2, 2024
3dfeec2
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Dec 3, 2024
e8629ca
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Dec 4, 2024
deeca6b
Code improvements. test still failing
gzanitti Dec 4, 2024
df5b7be
fix program columns to witness columns
gzanitti Dec 5, 2024
c859b60
Update pilopt/src/lib.rs
gzanitti Dec 10, 2024
42c08b2
Update pilopt/src/lib.rs
gzanitti Dec 10, 2024
934f668
minor changes
gzanitti Dec 10, 2024
1fd819f
main + more improvements
gzanitti Dec 10, 2024
7dd3a12
transitive substitutions
gzanitti Dec 10, 2024
c01dca9
equal_constrained_transitive
gzanitti Dec 10, 2024
926eedd
Merge remote-tracking branch 'upstream/main' into equal_constrained_opt
gzanitti Dec 11, 2024
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
94 changes: 94 additions & 0 deletions pilopt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub fn optimize<T: FieldElement>(mut pil_file: Analyzed<T>) -> Analyzed<T> {
extract_constant_lookups(&mut pil_file);
remove_constant_witness_columns(&mut pil_file);
simplify_identities(&mut pil_file);
remove_equal_constrained_witness_columns(&mut pil_file);
remove_trivial_identities(&mut pil_file);
remove_duplicate_identities(&mut pil_file);
remove_unreferenced_definitions(&mut pil_file);
Expand Down Expand Up @@ -620,3 +621,96 @@ fn remove_duplicate_identities<T: FieldElement>(pil_file: &mut Analyzed<T>) {
.collect();
pil_file.remove_identities(&to_remove);
}

fn equal_constrained<T: FieldElement>(
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
poly_data: (&u64, &AlgebraicExpression<T>),
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
) -> Option<((String, PolyID), (String, PolyID))> {
let (_, expression) = poly_data;

match expression {
AlgebraicExpression::BinaryOperation(AlgebraicBinaryOperation {
left,
op: AlgebraicBinaryOperator::Sub,
right,
}) => {
match (left.as_ref(), right.as_ref()) {
(
AlgebraicExpression::Reference(left_ref),
AlgebraicExpression::Reference(right_ref),
) => {
if left_ref.is_witness()
&& !left_ref.next
&& right_ref.is_witness()
&& !right_ref.next
{
// Choose which column to keep based on id

if left_ref.poly_id.id < right_ref.poly_id.id {
Some((
(left_ref.name.clone(), left_ref.poly_id),
(right_ref.name.clone(), right_ref.poly_id),
))
} else {
Some((
(right_ref.name.clone(), right_ref.poly_id),
(left_ref.name.clone(), left_ref.poly_id),
))
}
} else {
None
}
}
_ => None,
}
}
_ => None,
}
}

fn remove_equal_constrained_witness_columns<T: FieldElement>(pil_file: &mut Analyzed<T>) {
let substitutions = pil_file
.identities
.iter()
.filter_map(|id| {
if let Identity::Polynomial(PolynomialIdentity {
id: pid,
expression: e,
..
}) = id
{
equal_constrained((pid, e))
} else {
None
}
})
.collect::<Vec<_>>();

gzanitti marked this conversation as resolved.
Show resolved Hide resolved
let substitutions_by_id = substitutions
.iter()
.map(|((_, id_to_remove), to_keep)| (*id_to_remove, to_keep))
.collect::<HashMap<PolyID, _>>();

pil_file.post_visit_expressions_in_identities_mut(&mut |e: &mut AlgebraicExpression<_>| {
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
if let AlgebraicExpression::Reference(ref mut reference) = e {
if let Some((replacement_name, replacement_id)) =
substitutions_by_id.get(&reference.poly_id)
{
reference.poly_id = *replacement_id;
reference.name = replacement_name.clone();
}
}
});

let identities_to_remove = substitutions
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
.iter()
.map(|((_, pid), _)| pid.id as usize)
.collect::<BTreeSet<_>>();

let definitions_to_remove = substitutions
.into_iter()
.map(|((name, _), _)| name)
.collect::<BTreeSet<_>>();

pil_file.remove_definitions(&definitions_to_remove);
pil_file.remove_identities(&identities_to_remove);
}
4 changes: 1 addition & 3 deletions pilopt/tests/optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ fn replace_fixed() {
one * Y = zero * Y + 7 * X;
"#;
let expectation = r#"namespace N(65536);
col witness X;
col witness Y;
N::X = N::Y;
N::Y = 7 * N::X;
N::Y = 7 * N::Y;
"#;
let optimized = optimize(analyze_string::<GoldilocksField>(input).unwrap()).to_string();
assert_eq!(optimized, expectation);
Expand Down
Loading