gemini-code-assist[bot] commented on code in PR #19496:
URL: https://github.com/apache/tvm/pull/19496#discussion_r3177152662


##########
src/tirx/transform/common_subexpr_elim.cc:
##########
@@ -275,6 +282,13 @@ 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 wholly-constant expressions (no Var anywhere in the tree).
+    // BufferLoad is already filtered above by IsForbiddenNode, so
+    // "contains no Var" is sufficient to declare the expression a
+    // compile-time constant. Hoisting it adds noise; the constant
+    // folder will collapse it.
+    auto contains_var = [](const PrimExpr& e) { return e.as<VarNode>() != 
nullptr; };
+    if (!CheckContains::ExprContains(expr, contains_var)) return false;

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The `IsEligible` predicate now performs two separate full traversals of the 
expression tree (lines 284 and 291) for every node visited during the CSE 
planning phase. This leads to $O(N^2)$ complexity relative to the expression 
depth, which can be significant for large generated TIR programs.
   
   Furthermore, the check at line 282 is redundant because 
`CheckContains::ExprContains` at line 284 already checks the root node. 
Additionally, since `IsEligible` is only invoked from `RecordExpr` within 
specific `VisitExpr_` overrides (which exclude `Call` and `BufferLoad`), the 
root-level check at line 282 will always be false.
   
   Consider combining the 'forbidden node' and 'contains var' checks into a 
single pass or, ideally, caching these properties during the bottom-up 
traversal of `CSEPlanner` to maintain linear complexity.



-- 
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]

Reply via email to