Remove unused global data descriptor shapes from arguments #1338
Conversation
| for o in self.offset: | ||
| if isinstance(o, sp.Expr): | ||
| result |= set(o.free_symbols) | ||
|
|
||
| if self.transient or all_symbols: |
There was a problem hiding this comment.
Why is the transient check only here? What about strides and offset just above?
There was a problem hiding this comment.
Strides and offset are always necessary if you have any memlet (a[i*stride] will be generated in the code).
dace/sdfg/sdfg.py
Outdated
| for name, desc in self.arrays.items(): | ||
| defined_syms.add(name) | ||
| if not desc.transient: |
There was a problem hiding this comment.
I believe I understand the algorithm now. What I don't like is that it spills the need to specialize for Data subclasses to SDFG/SDFGState.free_symbols. Wouldn't it be cleaner not to have the transient check in Data.free_symbols and instead have it only here, without needing to specialize for strides/totalsize/etc.?
There was a problem hiding this comment.
To clarify, I would do:
for name, desc in self.arrays.items():
if not desc.transient:
not_strictly_necessary_global_symbols |= set(map(str, desc.free_symbols))where free_symbols is the current definition. Alternatively, if the new definitions of free/used symbols are necessary, it could be desc.used_symbols(all_symbols=True).
Is the solution chosen an optimization, e.g., we can safely rule out that we don't need to check the strides or something like that?
There was a problem hiding this comment.
You’re right and I started to rewrite it as such.
| 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.
Unrelated to the PR, but why is YAPF constantly switching the formatting of such lines?
There was a problem hiding this comment.
I think it depends on the version of yapf being used
This PR allows symbols that exist only in non-transient arrays to generate code successfully. This avoids the case of unused program arguments and enables compilation of more programs, while retaining symbolic analysis.