-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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] Avoid assigning range of possible values to integers #11859
Conversation
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.
Also CC: @vinx13 |
// If the conditional is comparing two integers, do not assign a | ||
// value to them. | ||
if (x.Eval().as<IntImmNode>()) { | ||
return {}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this check be done first? If it's true, none of the "make bound" code above has any effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would, except that the value of x
isn't set until Match
returns true. So it was a choice between copy/paste of the check for IntImmNode
within each conditional or potentially throwing out the result of MakeBound
. Since MakeBound
is a rather short constructor, I preferred keeping the condition in one spot.
…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.
…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.
…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.
Previously, in
ConstIntBoundAnalyzer
, entering a conditional such asif 2==0
could result in the expression2
being treated as having a known value of zero within the body of the conditional. Evaluating the range of expressions using2
in the body of the conditional could result in exceptions being thrown, such as evaluatingexpr / 2
while setting2
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.