[BugFix] Corrected when proving loop layout contains a fragment buffer layout#1708
[BugFix] Corrected when proving loop layout contains a fragment buffer layout#1708LeiWang1999 merged 1 commit intotile-ai:mainfrom
Conversation
|
👋 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 change removes explicit Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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
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: 1
🤖 Fix all issues with AI agents
In `@src/op/parallel.cc`:
- Around line 649-652: The comment above the ProveFragmentContains call is stale
and misleading because the call now omits the check_forward_index parameter
(defaulting to false); update or remove the comment to reflect that
forward-index checking is not performed when validating a loop against a buffer
fragment. Locate the comment near the ProveFragmentContains(candidate, fragment,
vars, indice_map_[buffer], analyzer_) invocation and either delete the lines
referencing check_forward_index=true or rewrite them to state that forward index
checking is intentionally disabled here and only used when comparing two loop
layouts.
| // check_forward_index=true: when validating loop layout against buffer | ||
| // fragment, we need to ensure physical indices match for correct code gen. | ||
| if (!ProveFragmentContains(candidate, fragment, vars, indice_map_[buffer], | ||
| analyzer_, /*check_forward_index=*/true)) { | ||
| analyzer_)) { |
There was a problem hiding this comment.
Stale comment contradicts updated code behavior.
The comment at lines 649-650 states check_forward_index=true is needed when validating loop layout against buffer fragment, but the code now omits this parameter (defaulting to false). Per the PR description ("Only prove loop contain loop we need index checking"), this appears intentional—forward index checking should only apply when comparing two loop layouts, not loop-vs-buffer.
Please update or remove the comment to reflect the corrected logic.
Suggested fix
auto fragment = T.layout_map[buffer].as<Fragment>().value();
- // check_forward_index=true: when validating loop layout against buffer
- // fragment, we need to ensure physical indices match for correct code gen.
+ // check_forward_index defaults to false: forward-index equality is only
+ // required when comparing two loop layouts, not loop-vs-buffer fragment.
if (!ProveFragmentContains(candidate, fragment, vars, indice_map_[buffer],
analyzer_)) {🤖 Prompt for AI Agents
In `@src/op/parallel.cc` around lines 649 - 652, The comment above the
ProveFragmentContains call is stale and misleading because the call now omits
the check_forward_index parameter (defaulting to false); update or remove the
comment to reflect that forward-index checking is not performed when validating
a loop against a buffer fragment. Locate the comment near the
ProveFragmentContains(candidate, fragment, vars, indice_map_[buffer], analyzer_)
invocation and either delete the lines referencing check_forward_index=true or
rewrite them to state that forward index checking is intentionally disabled here
and only used when comparing two loop layouts.
|
@regression-perf |
Performance Regression Test ReportTriggered by: @LeiWang1999 Results
Artifacts
|
Only prove loop contain loop we need index checking.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.