[Feature] Enhance Loop Unswitching with Let Binding and Condition Handling#1766
[Feature] Enhance Loop Unswitching with Let Binding and Condition Handling#1766LeiWang1999 merged 1 commit intotile-ai:mainfrom
Conversation
…dling - Updated the loop unswitching pass to support hoisting Let-bound variables alongside their conditions, improving optimization for loops with variable dependencies. - Modified the CallCheckerExcludingIf class to exclude matching If nodes when checking for CallNode presence, ensuring safety during unswitching. - Enhanced IfBranchReplacer to replace all instances of If nodes with matching conditions, streamlining the transformation process. - Added comprehensive tests to validate the new behavior, including scenarios with multiple Let-bound variables and identical conditions across If statements.
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
📝 WalkthroughWalkthroughThe pull request enhances the loop unswitching transformation to robustly handle multiple identical conditions and let-bound variables. The implementation broadens condition exclusion logic, introduces a let-binding hoisting mechanism, updates the IfBranchReplacer to consolidate matching conditions, and integrates hoisted let-bindings around transformed loop structures while maintaining SSA properties. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/transform/loop_unswitching.cc (1)
185-215:⚠️ Potential issue | 🟠 MajorHandle transitive let-binding dependencies when checking loop variance.
The current check only inspects one level of let bindings. If a condition uses
x,xdepends ony, andydepends on the loop var, the condition is incorrectly treated as invariant and can be hoisted. That breaks correctness for chained lets.🐛 Proposed fix: recursively follow let bindings when checking loop variance
bool UsesLoopVarThroughLetBindings( const PrimExpr &cond, const Var &loop_var, const std::unordered_map<const VarNode *, PrimExpr> *let_bindings) { - // Check if condition directly uses loop variable - if (UsesVar(cond, [&](const VarNode *v) { return v == loop_var.get(); })) { - return true; - } - - // Check if any Let-bound variable used in condition has a binding that uses - // the loop variable - if (let_bindings) { - bool uses_loop_var = false; - PostOrderVisit(cond, [&](const ObjectRef &obj) { - if (uses_loop_var) - return; - if (const auto *var_node = obj.as<VarNode>()) { - auto it = let_bindings->find(var_node); - if (it != let_bindings->end()) { - // Check if the bound expression uses the loop variable - if (UsesVar(it->second, - [&](const VarNode *v) { return v == loop_var.get(); })) { - uses_loop_var = true; - } - } - } - }); - if (uses_loop_var) { - return true; - } - } - return false; + class LoopVarChecker : public ExprVisitor { + public: + LoopVarChecker(const Var &loop_var, + const std::unordered_map<const VarNode *, PrimExpr> *bindings) + : loop_var_(loop_var.get()), let_bindings_(bindings) {} + + void VisitExpr_(const VarNode *op) final { + if (uses_loop_var_) return; + if (op == loop_var_) { + uses_loop_var_ = true; + return; + } + if (!let_bindings_ || visiting_.count(op)) return; + auto it = let_bindings_->find(op); + if (it != let_bindings_->end()) { + visiting_.insert(op); + VisitExpr(it->second); + } + } + + bool uses_loop_var_ = false; + + private: + const VarNode *loop_var_; + const std::unordered_map<const VarNode *, PrimExpr> *let_bindings_; + std::unordered_set<const VarNode *> visiting_; + }; + + LoopVarChecker checker(loop_var, let_bindings); + checker(cond); + return checker.uses_loop_var_; }
🧹 Nitpick comments (1)
testing/python/transform/test_tilelang_transform_loop_unswitching.py (1)
222-387: Consider adding a chained let-binding test case.The new cases cover direct let-bound invariance well. A test where the condition uses
x,xdepends ony, andydepends on the loop var would guard against transitive dependency regressions.
Summary by CodeRabbit
Release Notes