-
Notifications
You must be signed in to change notification settings - Fork 49
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
feat[next][dace]: GTIR-to-SDFG lowering of let-lambdas #1589
feat[next][dace]: GTIR-to-SDFG lowering of let-lambdas #1589
Conversation
…ace-fieldview-let_lambdas
…ace-fieldview-let_lambdas
…-fieldview-neighbors
…ace-fieldview-let_lambdas
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.
Other than a couple of minor python style issues looks fine to me, but I think someone else should be the main reviewer of this PR (@tehrengruber ? @philip-paul-mueller ?)
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_to_sdfg.py
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_to_sdfg.py
Outdated
Show resolved
Hide resolved
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.
I did not found anything major, I have two suggestions and one question.
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_builtin_translators.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_builtin_translators.py
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_to_sdfg.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_builtin_translators.py
Show resolved
Hide resolved
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.
There are some small things but afterwards we should be able to merge it.
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_builtin_translators.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_to_sdfg.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_to_sdfg.py
Show resolved
Hide resolved
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.
LGTM.
…DFG) (#1601) This PR proposes an alternative design for the lowering of let-lambdas to SDFG, than the design provided in #1589. The previous PR added a separate state for the definition of let-symbols: the disadvantage of that solution is that the generated SDFG does not clearly represent the data dependency. The alternative design proposed in this PR uses a nested SDFG to represent some local symbols in the lambda scope. The lambda function is lowered inside the nested SDFG. We rely on the simplify pass to remove unnecessary nested SDFGs.
This PR adds lowering of let-lambdas to DaCe SDFG.