gemini-code-assist[bot] commented on code in PR #19502:
URL: https://github.com/apache/tvm/pull/19502#discussion_r3178901343
##########
src/tirx/transform/common_subexpr_elim.cc:
##########
@@ -275,6 +281,14 @@ class CSEPlanner : public StmtExprVisitor {
if (IsForbiddenNode(expr)) return false;
if (expr.as<RampNode>() || expr.as<BroadcastNode>()) return false;
if (CheckContains::ExprContains(expr, IsForbiddenNode)) return false;
+ // Reject bool-typed expressions. Boolean predicates almost always feed an
+ // if / Select / assert, where reading the condition inline is clearer than
+ // going through a `cse_v: bool = (a < b)` temporary, and where downstream
+ // simplification (ProveCondition, branch elimination) benefits from seeing
+ // the predicate directly. BoolImm is already filtered above as an IntImm
+ // leaf, so this rule only affects compound bool expressions
+ // (LT/LE/GT/GE/EQ/NE/And/Or/Not/Cast-to-bool/Select-of-bool).
+ if (expr.dtype().is_bool()) return false;
Review Comment:
The check for boolean-typed expressions should be moved before the recursive
`CheckContains::ExprContains` call. `CheckContains` performs a full traversal
of the expression tree (O(N)), whereas checking the `dtype` is an O(1)
operation. By reordering these checks, we can avoid the expensive traversal for
all boolean predicates, which are common in TIR, thereby improving the
efficiency of the CSE pass.
```suggestion
// Reject bool-typed expressions. Boolean predicates almost always feed
an
// if / Select / assert, where reading the condition inline is clearer
than
// going through a `cse_v: bool = (a < b)` temporary, and where
downstream
// simplification (ProveCondition, branch elimination) benefits from
seeing
// the predicate directly. BoolImm is already filtered above as an IntImm
// leaf, so this rule only affects compound bool expressions
// (LT/LE/GT/GE/EQ/NE/And/Or/Not/Cast-to-bool/Select-of-bool).
if (expr.dtype().is_bool()) return false;
if (IsForbiddenNode(expr)) return false;
if (expr.as<RampNode>() || expr.as<BroadcastNode>()) return false;
if (CheckContains::ExprContains(expr, IsForbiddenNode)) return false;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
