Skip to content

[TVMScript][TIR] Clarify scope of BlockNode::iter_vars#12726

Merged
vinx13 merged 1 commit intoapache:mainfrom
Lunderberg:block_iter_vars_out_of_scope_for_predicate
Sep 9, 2022
Merged

[TVMScript][TIR] Clarify scope of BlockNode::iter_vars#12726
vinx13 merged 1 commit intoapache:mainfrom
Lunderberg:block_iter_vars_out_of_scope_for_predicate

Conversation

@Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented Sep 7, 2022

Previously, it was ambiguous whether BlockNode::iter_vars were in-scope for BlockRealizeNode::predicate. ConvertBlocksToOpaque treated them as in-scope, and applied a mapping from iter_vars to iter_values. Similarly, TVMScript printing places T.where statements below the T.axis statements, where T.axis definitions are in scope. However, BlockRealizeNode::SEqualReduce and BlockRealizeNode::SHashReduce do not visit the block and iter_vars until after visiting the predicate, placing the iter_vars out of scope.

This commit updates the printing of T.where to be above T.axis, and updates ConvertBlocksToOpaque to report an error if the predicate contains references to BlockNode::iter_vars. After this commit, these three usages all consistently treat
BlockNode::iter_vars as out of scope for BlockRealizeNode::predicate.

cc @Hzfengsy @junrushao1994

Previously, it was ambiguous whether `BlockNode::iter_vars` were
in-scope for `BlockRealizeNode::predicate`.  `ConvertBlocksToOpaque`
treated them as in-scope, and applied a mapping from `iter_vars` to
`iter_values`.  Similarly, TVMScript printing places `T.where`
statements below the `T.axis` statements, where `T.axis` definitions
are in scope.  However, `BlockRealizeNode::SEqualReduce` and
`BlockRealizeNode::SHashReduce` do not visit the block and `iter_vars`
until after visiting the predicate, placing the `iter_vars` out of
scope.

This commit updates the printing of `T.where` to be above `T.axis`,
and updates `ConvertBlocksToOpaque` to report an error if the
predicate contains references to `BlockNode::iter_vars`.  After this
commit, these three usages all consistently treat
`BlockNode::iter_vars` as out of scope for
`BlockRealizeNode::predicate`.
@Lunderberg
Copy link
Contributor Author

Opposite of #12724, with the same goal of clarifying when BlockNode::iter_vars are in-scope. From @vinx13, these are not intended to be in-scope for parameters in BlockRealizeNode, and so this PR enforces that design intent.

@vinx13 vinx13 merged commit 14999f8 into apache:main Sep 9, 2022
@Lunderberg Lunderberg deleted the block_iter_vars_out_of_scope_for_predicate branch September 9, 2022 19:48
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
Previously, it was ambiguous whether `BlockNode::iter_vars` were
in-scope for `BlockRealizeNode::predicate`.  `ConvertBlocksToOpaque`
treated them as in-scope, and applied a mapping from `iter_vars` to
`iter_values`.  Similarly, TVMScript printing places `T.where`
statements below the `T.axis` statements, where `T.axis` definitions
are in scope.  However, `BlockRealizeNode::SEqualReduce` and
`BlockRealizeNode::SHashReduce` do not visit the block and `iter_vars`
until after visiting the predicate, placing the `iter_vars` out of
scope.

This commit updates the printing of `T.where` to be above `T.axis`,
and updates `ConvertBlocksToOpaque` to report an error if the
predicate contains references to `BlockNode::iter_vars`.  After this
commit, these three usages all consistently treat
`BlockNode::iter_vars` as out of scope for
`BlockRealizeNode::predicate`.
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.

2 participants