Skip to content

[LLZK-373] Analysis Improvements#204

Merged
iangneal merged 8 commits intomainfrom
iangneal/plonky3-analysis-bugs
Nov 6, 2025
Merged

[LLZK-373] Analysis Improvements#204
iangneal merged 8 commits intomainfrom
iangneal/plonky3-analysis-bugs

Conversation

@iangneal
Copy link
Contributor

@iangneal iangneal commented Nov 5, 2025

  • General: Reduce the number of values stored in dataflow lattices (in both the SourceRef and IntervalAnalysis lattices) by looking up values in prior lattices rather than propagating all values arbitrarily. This dramatically improves performance for large circuits.
  • General: make the getSovler helper function return a mutable DataFlowSolver, which is required for most operations on the solver due to its internal caching.
  • Interval Analysis: update handling of cast operations.

@iangneal iangneal marked this pull request as draft November 5, 2025 06:04
@iangneal iangneal requested a review from a team November 5, 2025 06:05
@iangneal
Copy link
Contributor Author

iangneal commented Nov 5, 2025

Marking as a draft while I confirm this change doesn't cause any regressions in ZK Vanguard.

@shankarapailoor
Copy link
Contributor

So is the idea with the lattice change that a lattice at a location will only contain entries for values defined at that location? If the interval analysis infers that a value %foo has a refined interval then it will update the lattice at %foo's definition?

@iangneal
Copy link
Contributor Author

iangneal commented Nov 5, 2025

So is the idea with the lattice change that a lattice at a location will only contain entries for values defined at that location? If the interval analysis infers that a value %foo has a refined interval then it will update the lattice at %foo's definition?

Effectively, yes (the lattices also store their direct operands and the function argument values in the case of the interval analysis). I have a ticket for refactoring these analyses to use the MLIR sparse dataflow analysis (as it is likely ultimately better suited for these use cases), but there will be some more work in doing that (e.g., ensuring that symbol lookups function correctly and handling dependencies for non-result producing operations, which is part of why we initially used the dense analysis), so it is TODO for now.

Tickets: https://veridise.atlassian.net/browse/LLZK-371, https://veridise.atlassian.net/browse/LLZK-372

@iangneal iangneal marked this pull request as ready for review November 5, 2025 22:11
@iangneal
Copy link
Contributor Author

iangneal commented Nov 5, 2025

@Veridise/zk-tooling-developers Marked as ready to review based on integration testing with ZK Vanguard. This PR now blocks https://github.com/Veridise/Vanguard/pull/822.

@iangneal iangneal requested a review from tim-hoffman November 6, 2025 04:26
Copy link
Member

@tim-hoffman tim-hoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iangneal iangneal merged commit af591a2 into main Nov 6, 2025
10 checks passed
@iangneal iangneal deleted the iangneal/plonky3-analysis-bugs branch November 6, 2025 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants