[Sync] prune redundant back-edge sync pairs#653
[Sync] prune redundant back-edge sync pairs#653TaoTao-real wants to merge 3 commits intohw-native-sys:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request enables back-edge pruning in the RemoveRedundantSync pass by checking for redundant synchronization operations within loop bodies and tail spans. The feedback identifies a need for a bounds check when accessing the syncIR_ array to ensure safety. Additionally, the reviewer pointed out that the syncFinder state should be reset between the two CheckRepeatSync calls to ensure the analysis of the head and tail spans remains independent, preventing side effects from affecting the redundancy detection logic.
| auto *loopElement = | ||
| dyn_cast<LoopInstanceElement>(syncIR_[forEndIndex.value()].get()); |
There was a problem hiding this comment.
It is safer to verify that the index forEndIndex.value() is within the bounds of syncIR_ before accessing it, consistent with the safety checks used in CheckRepeatSync.
| auto *loopElement = | |
| dyn_cast<LoopInstanceElement>(syncIR_[forEndIndex.value()].get()); | |
| checkSyncIRIndex(syncIR_, forEndIndex.value()); | |
| auto *loopElement = | |
| dyn_cast<LoopInstanceElement>(syncIR_[forEndIndex.value()].get()); |
| return CheckRepeatSync(begin, loopElement->endId, syncFinder, setFlag) || | ||
| CheckRepeatSync(loopElement->beginId, end, syncFinder, setFlag); |
There was a problem hiding this comment.
The current implementation reuses the syncFinder state between the two CheckRepeatSync calls. If the first call (tail span) finds a set_flag but no matching wait_flag, those bits remain set when checking the second span (head span). This allows a set_flag in the tail to match a wait_flag in the head, effectively detecting loop-carried redundancies.
However, the PR description and code comments state that pruning should occur if either span independently contains a complete inner pair. To strictly adhere to this "either span" logic and avoid side effects between the two independent analysis spans, syncFinder should be reset before the second call.
if (CheckRepeatSync(begin, loopElement->endId, syncFinder, setFlag)) {
return true;
}
std::fill(syncFinder.begin(), syncFinder.end(), false);
return CheckRepeatSync(loopElement->beginId, end, syncFinder, setFlag);
Codex Review该评论由 review 机器人自动更新。
Summary未检查到 PR #653 存在问题 FindingsNo issues found. |
Summary
Motivation
Testing