Fix redundant sync pruning by pipe pair#627
Fix redundant sync pruning by pipe pair#627zhangstevenunity merged 1 commit intohw-native-sys:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request optimizes the RemoveRedundantSync pass by relaxing the criteria for pruning redundant synchronization operations. Specifically, it removes the requirement for matching depRootBuffers and allows pruning of multi-buffer synchronizations based on pipe pair semantics, provided the inner synchronization's eventIdNum is not greater than the outer one. The changes include updates to the design documentation, header comments, and the implementation in RemoveRedundantSync.cpp, along with a new regression test and updates to existing test expectations. I have no feedback to provide.
Codex Review该评论由 review 机器人自动更新。
SummaryBroadened pipe-pair pruning can incorrectly remove loop-tagged sync pairs that still drive loop-specific event allocation/backward-match generation. Findings
|
78db797 to
fdcd8cb
Compare
|
I re-checked this review finding against the current PR head ( The concern would be valid if this PR allowed redundant-sync pruning to scan around loop back-edges. In that case, a loop-tagged However, the current PR explicitly disables that path: if (begin < end) {
return CheckRepeatSync(begin, end, syncFinder, setFlag);
} else {
// Back-edge pruning is intentionally disabled in correctness-first mode.
(void)forEndIndex;
return false;
}So real loop-carried/back-edge pairs ( I also validated this on a clean worktree checked out at
The remaining risky shape would be a forward loop-tagged pair like: set_flag(P, Q, carry_forEnd);
...
set_flag(P, Q, local);
wait_flag(P, Q, local);
...
wait_flag(P, Q, carry_forEnd);But current InsertSync generation does not appear to produce such forward So I believe the review warning is a false positive for this PR as currently written. If we later re-enable circular/back-edge redundant-sync pruning, this concern should be revisited with a dedicated regression. |
A5 板测失败
日志尾部 |
A3 板测完成(有跳过)
|
Summary
Motivation
eventIdNum == 1hasSameSyncDepRoots(...)forEndIndex(srcPipe, dstPipe)path.Design
srcPipedstPipeeventIdNum(relatedSync->eventIdNum > setFlag->eventIdNumstill rejected)issue226_remove_redundant_pipe_pair.ptoto guard the intended behavior.Testing
test/lit/pto/issue226_remove_redundant_pipe_pair.ptotest/lit/pto/issue564_k_loop_mte1_mte2_wait_regression.ptoRisk / Rollback
fdcd8cb.