[TIR] Additional Stmt/Expr simplication rules #11373
Conversation
|
|
|
Good point, and I hadn't considered Nan. Looking through other operators, it looks like there are existing simplifications that would also change |
src/arith/rewrite_simplify.cc
Outdated
| if (IsIndexType(op->dtype)) { | ||
| // Index rules | ||
| // cancelation rules |
There was a problem hiding this comment.
The integer simplifcation should be fast path (and clean, hopefully without Nans), it would be better to put new rules in else branch even if it means some duplications, since (recursively) checking side effects is expensive.
Doing floating point simplification is probably fine, but given the possibility of introducing additional types, it might be safer to say something like
if (IsIndexType(op->dtype)) {
old rules
} else if (op->dtype.is_float())
new rules
}
There was a problem hiding this comment.
Sounds reasonable, and updated as requested.
- Enabled simplification of `A[i] = A[i] + 0` into no-op. This was a bug introduced in apache#9727, which applied this rewrite only to `A[i] = A[i]`, and not to statements which simplify to `A[i] = A[i]`. Regression test added to prevent reoccurrence of this bug. - Enabled simplification of `x - x` to zero for floating point types. Previously, this simplification was applied only for data types that could be used as buffer indices.
Lunderberg
force-pushed
the
tir_simplify_rules
branch
from
23bdc17 to
bd7e8cd
Compare
|
Retriggering CI following #11456. |
src/arith/rewrite_simplify.cc
Outdated
| // cancelation rules | ||
| TVM_TRY_REWRITE_IF(x - x, ZeroWithTypeLike(x), | ||
| SideEffect(x.Eval()) <= CallEffectKind::kReadState); | ||
| TVM_TRY_REWRITE_IF((x + y) - y, x, SideEffect(y.Eval()) <= CallEffectKind::kReadState); | ||
| TVM_TRY_REWRITE_IF((x + y) - x, y, SideEffect(x.Eval()) <= CallEffectKind::kReadState); | ||
| TVM_TRY_REWRITE_IF(x - (y + x), 0 - y, SideEffect(x.Eval()) <= CallEffectKind::kReadState); | ||
| TVM_TRY_REWRITE_IF(x - (x + y), 0 - y, SideEffect(x.Eval()) <= CallEffectKind::kReadState); | ||
|
|
There was a problem hiding this comment.
should this be removed since you've added the else branch?
There was a problem hiding this comment.
Thank you for the catch, and they are now removed.
These came about after investigating some simplifications that may be useful for handling of padding introduced during padded layout transformations.
Enabled simplification of
A[i] = A[i] + 0into no-op. This was a bug introduced in [TE][TIR] Implement layout transformations, non-flat memory buffers #9727, which applied this rewrite only toA[i] = A[i], and not to statements which simplify toA[i] = A[i]. Regression test added to prevent reoccurrence of this bug.Enabled simplification of
x - xto zero for floating point types. Previously, this simplification was applied only for data types that could be used as buffer indices.