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

Fix read_and_write_sets() for ControlFlowRegion #1920

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

edopao
Copy link
Collaborator

@edopao edopao commented Feb 3, 2025

The PruneConnectors transformation relies on read_and_write_sets() to identify connectors on a nested SDFG that refer to unused data containers. With the introduction of ControlFlowRegion nodes, the data containers can now be accessed by symbolic expressions used as conditions in such nodes. This case was not considered in baseline.

@edopao edopao requested a review from phschaad February 3, 2025 18:02
for edge in self.all_interstate_edges():
read_set |= edge.data.free_symbols & array_names

# By definition, data that is referenced by the conditions (branching condition,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment appears copied from somewhere else, does this really apply here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it does. We are removing from single_use_data the symbols used by condition expressions in control flow regions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But what is single_use_data here, in this context? It appears that all we are doing here - based on the code - is adding symbols used on control flow region's meta accesses (conditions, loop increments, etc.) to the read_set of the SDFG?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now I get it, the misleading information is "single use data". I will change the code comment.

@edopao edopao requested a review from phschaad February 4, 2025 08:16
Co-authored-by: Philip Mueller <[email protected]>
Copy link
Collaborator

@phschaad phschaad left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM!

@phschaad phschaad enabled auto-merge February 4, 2025 08:31
@phschaad phschaad added this pull request to the merge queue Feb 4, 2025
Merged via the queue into spcl:main with commit 118c131 Feb 4, 2025
9 checks passed
@edopao edopao deleted the fix-prune_connectors branch February 4, 2025 13:52
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