Skip to content

[NFC] Refactor inline structs pass to prepare for handling more "remaining uses"#218

Merged
tim-hoffman merged 4 commits intomainfrom
th/refactor_inline_structs_pass
Nov 17, 2025
Merged

[NFC] Refactor inline structs pass to prepare for handling more "remaining uses"#218
tim-hoffman merged 4 commits intomainfrom
th/refactor_inline_structs_pass

Conversation

@tim-hoffman
Copy link
Member

@tim-hoffman tim-hoffman commented Nov 14, 2025

No Functional Changes

Preparing to fix a bug found in llzk-inline-structs pass... Refactoring handleRemainingUses() into a class to more clearly split up implementation into functions and avoid passing around the common data. Also added some debug output, clarified/added some documentation, and fixed a couple typos.

Empty changelog because it's only private API and doc changes.

@tim-hoffman tim-hoffman force-pushed the th/refactor_inline_structs_pass branch from e71c786 to db1bf09 Compare November 14, 2025 22:26
@tim-hoffman tim-hoffman requested a review from a team November 14, 2025 22:28
Copy link
Contributor

@iangneal iangneal left a comment

Choose a reason for hiding this comment

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

LGTM, just a note for the future.

// Find the write op that stores the created value
if (writeOp.getVal() == writtenValue) {
if (foundWrite) {
// Note: There is no reason for a subcomponent to be stored to more than one field.
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems we don't have this as a semantic check, since code like this is produced by the zirgen frontend and doesn't cause issues in the up-to-date llzk-opt. We might want to consider either adding one (if we want this to always be enforced) or adding a transformation pass for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Just a note for later, I know that's not the purpose of this PR).

@tim-hoffman tim-hoffman merged commit 9db8c23 into main Nov 17, 2025
10 checks passed
@tim-hoffman tim-hoffman deleted the th/refactor_inline_structs_pass branch November 17, 2025 17:34
raghav198 pushed a commit that referenced this pull request Nov 20, 2025
…ining uses" (#218)

* add debug output, clarify docs, fix typos
* refactoring to prepare for handling more "remaining uses"
* update debug output with new function names
raghav198 added a commit that referenced this pull request Dec 9, 2025
* Registered new transformation pass

* Concatenation pass works

* deleted comments

* Basic inlining complete

* Fixed issue calling @Product

* Refactoring, added root-struct option

* Aligning subfunctions

* Refactor to make struct inlining available on demand (#212)

* For now, fail when we can't align some calls

* [NFC] Refactor inline structs pass to prepare for handling more "remaining uses" (#218)

* add debug output, clarify docs, fix typos
* refactoring to prepare for handling more "remaining uses"
* update debug output with new function names

* Brought back LightweightSignalEquivalenceAnalysis

* Added testcase

* Shankara/pcl include fix (#221)

* used the wrong cmake flag when guarding PCL imports

* Added libAnalysis to CMake

* Removed libAnalysis from CMake

* Namespacing

* Namespacing

* Put analysis back

* Link `LLZKAnalysis` in `LLZKTransforms` (#225)

also explicitly link LLZKDialect in the dialects that use it to fix linker error that surfaced as a result

* remove redundant "add_dependencies" from cmake (#226)

* clang-format

* Changelog

* FORMAT

* Addressing comments

* update manual build instructions to build clang dependency

* Addressing comments

* Add missing headers to `llzk-tblgen` lib (#239)

* Refactor to add calleeContainsWitnessGen() and getSingleResultTypeOfWitnessGen() instead of overloading ...Compute()

* nit

* revert

---------

Co-authored-by: Raghav Malik <raghav@veridise.com>
Co-authored-by: Timothy Hoffman <4001421+tim-hoffman@users.noreply.github.com>
Co-authored-by: shankarapailoor <shankarapailoor@gmail.com>
Co-authored-by: Tim Hoffman <timothy.hoffman@veridise.com>
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.

2 participants