-
Notifications
You must be signed in to change notification settings - Fork 116
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
Remove unused global data descriptor shapes from arguments #1338
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
0040aad
Remove unused global data descriptor shapes from arguments
tbennun 1b94242
Fix implicit addition of symbols to SDFG based on data descriptors
tbennun 8efaeea
Merge remote-tracking branch 'origin/master' into codegen-necessarysyms
tbennun da1b31f
Replaced a use of `free_symbols` which should be `used_symbols`
tbennun 901a6db
Make test use pytest and use proper SDFG API
tbennun c9f454d
Fix test
tbennun 988d79f
Switch to used_symbols in arglist instead of changing free_symbols
tbennun ac47f8e
Merge remote-tracking branch 'origin/master' into codegen-necessarysyms
tbennun ae22d9c
Revert "Fix test"
tbennun 3dc6a34
Revert some now-unnecessary changes
tbennun b4deffe
Revert adding too many free symbols
tbennun 7388480
Merge branch 'master' into codegen-necessarysyms
tbennun File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
if TYPE_CHECKING: | ||
import dace.sdfg.graph | ||
|
||
|
||
@make_properties | ||
class Memlet(object): | ||
""" Data movement object. Represents the data, the subset moved, and the | ||
|
@@ -176,15 +177,16 @@ def to_json(self): | |
@staticmethod | ||
def from_json(json_obj, context=None): | ||
ret = Memlet() | ||
dace.serialize.set_properties_from_json(ret, | ||
json_obj, | ||
context=context, | ||
ignore_properties={'src_subset', 'dst_subset', 'num_accesses', 'is_data_src'}) | ||
|
||
dace.serialize.set_properties_from_json( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to the PR, but why is YAPF constantly switching the formatting of such lines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it depends on the version of yapf being used |
||
ret, | ||
json_obj, | ||
context=context, | ||
ignore_properties={'src_subset', 'dst_subset', 'num_accesses', 'is_data_src'}) | ||
|
||
# Allow serialized memlet to override src/dst_subset to disambiguate self-copies | ||
if 'is_data_src' in json_obj['attributes']: | ||
ret._is_data_src = json_obj['attributes']['is_data_src'] | ||
|
||
if context: | ||
ret._sdfg = context['sdfg'] | ||
ret._state = context['sdfg_state'] | ||
|
@@ -510,18 +512,30 @@ def validate(self, sdfg, state): | |
if self.data is not None and self.data not in sdfg.arrays: | ||
raise KeyError('Array "%s" not found in SDFG' % self.data) | ||
|
||
@property | ||
def free_symbols(self) -> Set[str]: | ||
""" Returns a set of symbols used in this edge's properties. """ | ||
def used_symbols(self, all_symbols: bool) -> Set[str]: | ||
""" | ||
Returns a set of symbols used in this edge's properties. | ||
|
||
:param all_symbols: If False, only returns the set of symbols that will be used | ||
in the generated code and are needed as arguments. | ||
""" | ||
# Symbolic properties are in volume, and the two subsets | ||
result = set() | ||
result |= set(map(str, self.volume.free_symbols)) | ||
if all_symbols: | ||
result |= set(map(str, self.volume.free_symbols)) | ||
if self.src_subset: | ||
result |= self.src_subset.free_symbols | ||
|
||
if self.dst_subset: | ||
result |= self.dst_subset.free_symbols | ||
|
||
return result | ||
|
||
@property | ||
def free_symbols(self) -> Set[str]: | ||
""" Returns a set of symbols used in this edge's properties. """ | ||
return self.used_symbols(all_symbols=True) | ||
|
||
def get_free_symbols_by_indices(self, indices_src: List[int], indices_dst: List[int]) -> Set[str]: | ||
""" | ||
Returns set of free symbols used in this edges properties but only taking certain indices of the src and dst | ||
|
@@ -640,6 +654,7 @@ class MemletTree(object): | |
all siblings of the same edge and their children, for instance if | ||
multiple inputs from the same access node are used. | ||
""" | ||
|
||
def __init__(self, | ||
edge: 'dace.sdfg.graph.MultiConnectorEdge[Memlet]', | ||
downwards: bool = True, | ||
|
This file contains 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the transient check only here? What about strides and offset just above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strides and offset are always necessary if you have any memlet (a[i*stride] will be generated in the code).