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

[LowerXMR] iterate to handle ref.sub . #5732

Merged
merged 3 commits into from
Jul 31, 2023

Conversation

dtzSiFive
Copy link
Contributor

Continue to handle everything else in a single walk.

Walking the ref dataflow directly would eliminate the need for this iteration, but finding the "roots" requires an IR walk anyway so just handle everything we can during a single walk and for any ref.sub's encountered handle them separately.

The outer iteration loop is bounded by the number of ref.sub's chained through backedges to storage (ref.define to storage (ports/instance results)), which is expected to be very small in practice, at worst O(ports).

Continue to handle everything else in a single walk.

Walking the ref dataflow directly would eliminate the need
for this iteration, but finding the "roots" requires an IR
walk anyway so just handle everything we can during a single walk
and for any ref.sub's encountered handle them separately.

The outer iteration loop is bounded by the number of ref.sub's chained through
`backedges` to storage (`ref.define` to storage (ports/instance results)),
which is expected to be very small in practice, at worst O(ports).
Add test for multi-iteration (not just post-walk)
that fails without this fix.

Per reviewer feedback, thanks!
@dtzSiFive dtzSiFive requested a review from prithayan July 31, 2023 16:31
Copy link
Contributor

@prithayan prithayan left a comment

Choose a reason for hiding this comment

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

Looks good

@dtzSiFive dtzSiFive merged commit 8c83a96 into llvm:main Jul 31, 2023
5 checks passed
@dtzSiFive dtzSiFive deleted the feature/lowerxmr-refsub-iterate branch July 31, 2023 21:49
@dtzSiFive
Copy link
Contributor Author

Thanks, @prithayan !

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.

None yet

2 participants