fix[next-dace]: Reject MoveDataflowIntoIfBody when tasklet free symbol clashes with inner data descriptor#2614
Open
edopao wants to merge 1 commit into
Conversation
…bol clashes with an inner data descriptor A tasklet being relocated may reference a symbol from the outer SDFG as a free variable (no input connector). If the nested SDFG already contains a data descriptor with the same name, the relocated tasklet would silently read that inner descriptor instead of the outer symbol, producing wrong results. Fix _check_for_data_and_symbol_conflicts to return False whenever any symbol in required_symbols shares its name with a data descriptor in the nested SDFG. A regression test is included. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness issue in the DaCe MoveDataflowIntoIfBody transformation where relocating a tasklet that references an outer SDFG symbol as a free variable could silently change semantics if the nested SDFG already contains a data descriptor with the same name.
Changes:
- Reject relocation when any symbol required by the relocatable subgraph conflicts with an existing data descriptor name inside the nested SDFG.
- Add a regression test covering the “free symbol name clashes with inner scalar descriptor” case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/gt4py/next/program_processors/runners/dace/transformations/move_dataflow_into_if_body.py |
Adds an additional safety check in _check_for_data_and_symbol_conflicts to prevent symbol/data name shadowing after relocation. |
tests/next_tests/unit_tests/program_processor_tests/runners_tests/dace_tests/transformation_tests/test_move_dataflow_into_if_body.py |
Adds a regression test ensuring the transformation refuses to apply when a tasklet’s free symbol would clash with an inner data descriptor name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MoveDataflowIntoIfBodycould relocate a tasklet whose code references an outer SDFG symbol as a free variable (no input connector), even when the nested SDFG already has a data descriptor with the same name. After relocation the tasklet would silently read the inner descriptor instead of the outer symbol, producing incorrect results._check_for_data_and_symbol_conflictsnow returnsFalsewhen any name inrequired_symbolsmatches an existing data descriptor inif_block.sdfg.test_if_mover_symbol_clashes_with_inner_dataadded totest_move_dataflow_into_if_body.py.Test plan
test_if_mover_symbol_clashes_with_inner_datafails before the fix and passes after.test_move_dataflow_into_if_body.pycontinue to pass.🤖 Generated with Claude Code