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][Arith] Additional Simplifications Inside Conditionals #11524

Merged
merged 9 commits into from
Jun 2, 2022

Conversation

Lunderberg
Copy link
Contributor

This adds additional simplifications to RewriteSimplifier while inside a conditional statement. These are simplifications that are expected to be necessary for simplifying conditionals for padding buffer transforms. The main additions are below:

  • Use of RewriteSimplifier::literal_constraints_ for simplifications of boolean expressions. Previously, this was only used for simplifications inside a tir.likely annotation.
  • Use of RewriteSimplifier::literal_constraints_ to identify known false constraints. This allows a constraint of i!=n to simplify i==n to false.
  • Use of EqNode constraints in ConstIntBoundAnalyzer. Previously, inequalities were used as comparisons. With this change an i==5 constraint be used to reduce the bounds of i to [5,5].

Previously, constraints with inequalities were recognized and used for
simplifications by `ConstIntBoundAnalyzer` and `ModularSetAnalyzer`,
but constraints with equalities were not.  This adds equality-based
constraints.  (e.g. Inside the then-case of `if i==5`, the value of
`i` is known to be 5.)
Previously, constraints were only checked within a `tir.likely`
annotation.  After this change, constraints are used for
simplification of all boolean expressions.  (e.g. Within a conditional
`if i==n`, the expression `(i==n) and (j==m)` can be simplified to
`j==m`.)
If a literal constraint relies on the contents of a buffer, the
constraint may not be assumed to hold.  This prevents the incorrect
rewriting of `A[i]==n` to true within a `if A[i]==n` conditional, as
the value of `A[i]` may have changed.
Inside a constraint `if i==n and j==m`, both `i==n` and `j==m` may be
replaced with true, even in separate expressions.

This commit uses a new internal utility function
`tvm::arith::ExtractConstraints`, which breaks up a boolean expression
into a list of true statements.  This may be used to reduce
duplication elsewhere, such as `const_int_bound.cc` and
`iter_affine_map.cc`.
When inside a conditional of `i!=n`, in addition to the previous
replacement of `i!=n` with true, we can also replace `i==n` with
false.
@Lunderberg Lunderberg requested a review from vinx13 June 1, 2022 17:18
@tqchen
Copy link
Member

tqchen commented Jun 1, 2022

@vinx13 vinx13 merged commit c78539c into apache:main Jun 2, 2022
@Lunderberg Lunderberg deleted the arith_simplify_constraints branch June 3, 2022 14:00
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 23, 2022
Previously, in `ConstIntBoundAnalyzer`, entering a conditional such as
`if 2==0` could result in the expression `2` being treated as having a
known value of zero within the body of the conditional.  Evaluating
the range of expressions using `2` in the body of the conditional
could result in exceptions being thrown, such as evaluating `expr / 2`
while setting `2` to its maximum value of zero.

This issue was present for conditions with inequalities for some time,
but was introduced for conditions with equalities in
apache#11524.  Both types are resolved in
this PR.
vinx13 pushed a commit that referenced this pull request Jun 24, 2022
…1859)

Previously, in `ConstIntBoundAnalyzer`, entering a conditional such as
`if 2==0` could result in the expression `2` being treated as having a
known value of zero within the body of the conditional.  Evaluating
the range of expressions using `2` in the body of the conditional
could result in exceptions being thrown, such as evaluating `expr / 2`
while setting `2` to its maximum value of zero.

This issue was present for conditions with inequalities for some time,
but was introduced for conditions with equalities in
#11524.  Both types are resolved in
this PR.
zxybazh pushed a commit to zxybazh/tvm that referenced this pull request Jun 26, 2022
…ache#11859)

Previously, in `ConstIntBoundAnalyzer`, entering a conditional such as
`if 2==0` could result in the expression `2` being treated as having a
known value of zero within the body of the conditional.  Evaluating
the range of expressions using `2` in the body of the conditional
could result in exceptions being thrown, such as evaluating `expr / 2`
while setting `2` to its maximum value of zero.

This issue was present for conditions with inequalities for some time,
but was introduced for conditions with equalities in
apache#11524.  Both types are resolved in
this PR.
blackkker pushed a commit to blackkker/tvm that referenced this pull request Jul 7, 2022
…ache#11859)

Previously, in `ConstIntBoundAnalyzer`, entering a conditional such as
`if 2==0` could result in the expression `2` being treated as having a
known value of zero within the body of the conditional.  Evaluating
the range of expressions using `2` in the body of the conditional
could result in exceptions being thrown, such as evaluating `expr / 2`
while setting `2` to its maximum value of zero.

This issue was present for conditions with inequalities for some time,
but was introduced for conditions with equalities in
apache#11524.  Both types are resolved in
this PR.
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
…ache#11859)

Previously, in `ConstIntBoundAnalyzer`, entering a conditional such as
`if 2==0` could result in the expression `2` being treated as having a
known value of zero within the body of the conditional.  Evaluating
the range of expressions using `2` in the body of the conditional
could result in exceptions being thrown, such as evaluating `expr / 2`
while setting `2` to its maximum value of zero.

This issue was present for conditions with inequalities for some time,
but was introduced for conditions with equalities in
apache#11524.  Both types are resolved in
this PR.
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.

3 participants