Skip to content

[TIR][Fix] Handle iter vars in block predicate for SHash/SEqual#12724

Closed
Lunderberg wants to merge 1 commit intoapache:mainfrom
Lunderberg:block_node_predicate_iter_vars
Closed

[TIR][Fix] Handle iter vars in block predicate for SHash/SEqual#12724
Lunderberg wants to merge 1 commit intoapache:mainfrom
Lunderberg:block_node_predicate_iter_vars

Conversation

@Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented Sep 7, 2022

When lowering a non-opaque TIR block to an opaque block in tir.transform.ConvertBlocksToOpaque, the iter vars in BlockNode::iter_varsare replaced by their values inBlockRealizeNode::iter_valueswherever they occur inBlockRealize. However, prior to this commit, the BlockRealizeNode::predicatewas visited before the variable definition ofBlockNode::iter_vars. As a result, StructuralHash and StructuralEqual would treat any occurrences as undefined variables, which is inconsistent with how they are handled in ConvertBlocksToOpaque`.

This commit resolves the inconsistency by changing the visit order such that BlockRealizeNode::block is visited before BlockRealizeNode::predicate.

cc @Hzfengsy @junrushao1994

When lowering a non-opaque TIR block to an opaque block in
`tir.transform.ConvertBlocksToOpaque`, the iter vars in
`BlockNode::iter_vars` are replaced by their values in
`BlockRealizeNode::iter_values` wherever they occur in `BlockRealize`.
However, prior to this commit, the `BlockRealizeNode::predicate` was
visited before the variable definition of `BlockNode::iter_vars`.  As
a result, StructuralHash and StructuralEqual would treat any
occurrences as undefined variables, which is inconsistent with how
they are handled in `ConvertBlocksToOpaque`.

This commit resolves the inconsistency by changing the visit order
such that `BlockRealizeNode::block` is visited before
`BlockRealizeNode::predicate`.
@Lunderberg
Copy link
Contributor Author

Closing PR. From @vinx13 , it is not intended behavior for the predicate to use the iter_vars.

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.

1 participant