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

Support attributes in symbolic expressions #1369

Merged
merged 14 commits into from
Sep 28, 2023
Merged

Support attributes in symbolic expressions #1369

merged 14 commits into from
Sep 28, 2023

Conversation

tbennun
Copy link
Collaborator

@tbennun tbennun commented Sep 14, 2023

I do not know how to make tests for that.

@tbennun tbennun requested a review from alexnick83 September 14, 2023 15:18
Copy link
Contributor

@alexnick83 alexnick83 left a comment

Choose a reason for hiding this comment

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

We will need updates for multi-nested Structures, but let's do it step by step.

dace/codegen/targets/cpu.py Outdated Show resolved Hide resolved
dace/codegen/targets/cpu.py Outdated Show resolved Hide resolved
@@ -405,6 +405,9 @@ def _infer_dtype(t: Union[ast.Name, ast.Attribute]):

def _Attribute(t, symbols, inferred_symbols):
inferred_type = _dispatch(t.value, symbols, inferred_symbols)
if (isinstance(inferred_type, dtypes.pointer) and isinstance(inferred_type.base_type, dtypes.struct) and
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@alexnick83 what about plain structs (i.e., not pointers)

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have decided that Structures will always resolve to a pointer to a struct, I think it is better to not handle that case (now), since it might be something unrelated to nested data and cause further confusion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Note: This assumption was valid for the time being, but I am now arguing to change that assumption. For cases where the type of an expression is being evaluated, where the t.value type of an Attribute expression is not merely a structure, but an array of structures (indexed through a Slice), the returned inferred_type is no longer a pointer but directly a plain struct.

Example: my_array_of_structs[i].my_attribute

Commit e2561ed implements a handle for this additional case.

@alexnick83 alexnick83 merged commit 3e73304 into master Sep 28, 2023
9 checks passed
@tbennun tbennun deleted the sym-attr branch February 5, 2024 18:21
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.

3 participants