Skip to content

[BugFix] Corrected when proving loop layout contains a fragment buffer layout #1708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LeiWang1999 merged 1 commit into from
Jan 21, 2026
Merged

[BugFix] Corrected when proving loop layout contains a fragment buffer layout #1708

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/op/parallel.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ bool ParallelOpNode::ValidateCandidateAgainstFragments(
// 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_)) {
Comment on lines 649 to +652
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

return false;
}
}
Expand Down Expand Up @@ -815,7 +815,8 @@ ParallelOpNode::ChooseBestCandidate(const Fragment &candidate_from_buffer,
auto contains = [&](const Fragment &big, const Fragment &small) {
// contains(A, B) means: for any loop index, the threads that access
// B's elements are a subset of those that access A's elements.
return ProveFragmentContains(small, big, vars, vars, analyzer_);
return ProveFragmentContains(small, big, vars, vars, analyzer_,
/*check_forward_index=*/true);
};

bool buf_ok = ValidateCandidateAgainstFragments(candidate_from_buffer, T);
Expand Down
Loading