feat[next]: Refactor CSE pass to support ITIR.Program (GTIR branch) #1579
Conversation
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
tehrengruber
commented
Jul 18, 2024
| @@ -264,25 +264,23 @@ def visit_FunCall(self, node: gtfn_ir.FunCall, **kwargs: Any) -> gtfn_ir_common. | |||
| self.imp_list_ir.append(InitStmt(lhs=gtfn_ir_common.Sym(id=f"{lam_idx}"), rhs=node)) | |||
| return gtfn_ir_common.SymRef(id=f"{lam_idx}") | |||
| if isinstance(node.fun, gtfn_ir.Lambda): | |||
There was a problem hiding this comment.
The scoping here was completely broken leading to multiple SymbolName's with the same id being declared in InitStmt. The fix is ok, but not great.
egparedes
reviewed
Jul 19, 2024
| @@ -365,10 +371,10 @@ class CommonSubexpressionElimination(PreserveLocationVisitor, NodeTranslator): | |||
| Perform common subexpression elimination. | |||
|
|
|||
There was a problem hiding this comment.
Could you briefly outline here the algorithm and the used tools (e.g. other visitors, functions, ...)
| @@ -116,15 +116,17 @@ def _(lhs: ts.ScalarType, rhs: ts.ScalarType) -> ts.ScalarType | ts.TupleType: | |||
|
|
|||
|
|
|||
| @_register_builtin_type_synthesizer | |||
| def deref(it: it_ts.IteratorType) -> ts.DataType: | |||
| def deref(it: it_ts.IteratorType) -> ts.DataType | ts.DeferredType: | |||
There was a problem hiding this comment.
Just a question: is the type annotation for it correct? Shouldn't it be a union with DeferredType? If that is the case, why is not type checker complaining here?
Suggested change
| def deref(it: it_ts.IteratorType) -> ts.DataType | ts.DeferredType: | |
| def deref(it: it_ts.IteratorType| ts.DeferredType) -> ts.DataType | ts.DeferredType: |
src/gt4py/next/program_processors/codegens/gtfn/gtfn_ir_to_gtfn_im_ir.py
Outdated
Show resolved
Hide resolved
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.
Extends the common subexpression elimination to support the new
itir.Programnode and pushes the intermediateFencil->Programconversion upwards the pass manager. The CSE pass now uses the type inference such that only field expressions or composites thereof are collected in field-view context (i.e. outside ofas_fieldop).