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

[TIR] HoistExpression, generalization of HoistIfThenElse #11592

Merged
merged 12 commits into from
Jun 24, 2022

Conversation

Lunderberg
Copy link
Contributor

This is a generalized form of HoistIfThenElse, which can also hoist Let bindings and independent portions of conditional expressions, in addition to hoisting an entire conditional expression. This will be used in upcoming changes to separate compute loops into a slow loop that handles edge cases and a fast branchless loop.

This feels like it should definitely be part of RewriteSimplify, but
that will require making CanInlineLet be a virtual function.
This is a generalized form of HoistIfThenElse, which can also hoist
Let bindings, or portions of conditional expressions.  This will be
used in upcoming changes to separate compute loops into a slow loop
that handles edge cases and a fast branchless loop.
@Lunderberg Lunderberg requested a review from vinx13 June 6, 2022 15:10
@Lunderberg
Copy link
Contributor Author

Current failures in test_sort are due to a thread_extent being provable on one side of a hoisted condition, but not on the other. These failures are resolved in #11646, which introduces simplifications for ceil and log2, along with bounds propagation of operator<<, so that the thread_extent is no longer a conflict.

Once #11646 is merged, will rebase this PR on top of it.

@Lunderberg
Copy link
Contributor Author

Merged from main now that #11646 is merged. Previous CI failures were due to inconsistent simplification of ceil(log2(x)) on each side of an if/else block, which (hopefully) should resolve the remaining CI failures.

Didn't correctly reproduce previous behavior.  In addition to
preventing hoisting of expressions that use a block
variable (e.g. threadIdx.x), should also prevent hoisting of
expressions across a "thread_extent" AttrStmt.
include/tvm/tir/transform.h Outdated Show resolved Hide resolved
@vinx13 vinx13 merged commit 9aaae28 into apache:main Jun 24, 2022
zxybazh pushed a commit to zxybazh/tvm that referenced this pull request Jun 26, 2022
* [TIR][Arith] Use non-inlined bindings when proving conditional

* [TIR][Arith] Recognize Var when used as a literal constraint

* [TIR][Arith] Added simplification of constrained if_then_else op

This feels like it should definitely be part of RewriteSimplify, but
that will require making CanInlineLet be a virtual function.

* [TIR] Implemented HoistExpression transformation

This is a generalized form of HoistIfThenElse, which can also hoist
Let bindings, or portions of conditional expressions.  This will be
used in upcoming changes to separate compute loops into a slow loop
that handles edge cases and a fast branchless loop.

* [TIR] Expressed HoistIfThenElse as special case of HoistExpression

* Lint fixes

* Fixed breakage in tvmc unit test that relied on pass type

* More accurate handling of kUsingBlockVar

Didn't correctly reproduce previous behavior.  In addition to
preventing hoisting of expressions that use a block
variable (e.g. threadIdx.x), should also prevent hoisting of
expressions across a "thread_extent" AttrStmt.

* Updated comment for HoistExpression pass

* Fix linting error
@Lunderberg Lunderberg deleted the hoist_if_let branch June 27, 2022 14:28
blackkker pushed a commit to blackkker/tvm that referenced this pull request Jul 7, 2022
* [TIR][Arith] Use non-inlined bindings when proving conditional

* [TIR][Arith] Recognize Var when used as a literal constraint

* [TIR][Arith] Added simplification of constrained if_then_else op

This feels like it should definitely be part of RewriteSimplify, but
that will require making CanInlineLet be a virtual function.

* [TIR] Implemented HoistExpression transformation

This is a generalized form of HoistIfThenElse, which can also hoist
Let bindings, or portions of conditional expressions.  This will be
used in upcoming changes to separate compute loops into a slow loop
that handles edge cases and a fast branchless loop.

* [TIR] Expressed HoistIfThenElse as special case of HoistExpression

* Lint fixes

* Fixed breakage in tvmc unit test that relied on pass type

* More accurate handling of kUsingBlockVar

Didn't correctly reproduce previous behavior.  In addition to
preventing hoisting of expressions that use a block
variable (e.g. threadIdx.x), should also prevent hoisting of
expressions across a "thread_extent" AttrStmt.

* Updated comment for HoistExpression pass

* Fix linting error
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
* [TIR][Arith] Use non-inlined bindings when proving conditional

* [TIR][Arith] Recognize Var when used as a literal constraint

* [TIR][Arith] Added simplification of constrained if_then_else op

This feels like it should definitely be part of RewriteSimplify, but
that will require making CanInlineLet be a virtual function.

* [TIR] Implemented HoistExpression transformation

This is a generalized form of HoistIfThenElse, which can also hoist
Let bindings, or portions of conditional expressions.  This will be
used in upcoming changes to separate compute loops into a slow loop
that handles edge cases and a fast branchless loop.

* [TIR] Expressed HoistIfThenElse as special case of HoistExpression

* Lint fixes

* Fixed breakage in tvmc unit test that relied on pass type

* More accurate handling of kUsingBlockVar

Didn't correctly reproduce previous behavior.  In addition to
preventing hoisting of expressions that use a block
variable (e.g. threadIdx.x), should also prevent hoisting of
expressions across a "thread_extent" AttrStmt.

* Updated comment for HoistExpression pass

* Fix linting error
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