fix: guard co-live multi-tile slots in InsertSync - #1132
Merged
Conversation
zhangstevenunity
marked this pull request as ready for review
August 5, 2026 01:26
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR #1132 fixed two of the three places that treat a `set_flag_dyn` / `wait_flag_dyn` pair as whole-pipe ordering, but left `RemoveRedundantSync::CanMatchedSync` untouched. That is now the sole remaining suppressor: with two distinct `pto.alloc_multi_tile` regions in one loop body the analysis correctly produces both back-edge pairs, and redundancy removal then deletes the second one, so the second region's tload ships with no WAR guard against the previous iteration's reader. A slot-keyed pair rendezvouses on the event lane `slotSSAExpr % slotCount`, so it orders only the accesses that land on that lane. Apply the rule one-directionally: such a pair may BE covered by a whole-pipe pair, which is strictly stronger, but may never PROVIDE coverage. Static-covers-dyn elimination and every existing static-covers-static decision are unchanged. Hoist the predicate into SyncCommon.h as `isLaneKeyedBackEdgeSync` and route PR #1132`s two inlined copies through it so there is one definition. Measured on a3/level2: - two distinct regions: each keeps its own correctly keyed dyn pair on its own lane group (ids {0,1} and {2,3}); the wait and set of each pair share one slot expression, so the N prologue primes stay balanced. - the same-region two-gets shape from #1132 is unchanged: still one static V -> MTE2 pair, and the redundant second static pair is still removed. - event-id pressure unchanged on five_regions; the pipe_barrier(PIPE_ALL) exhaustion cliff is not reached any earlier. lit: 1609 discovered, 1608 passed, 1 unsupported, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The predicate tests `eventIdNum > 1 && slotSSAExpr`; it never checks for a back edge, so the old name asserted a property the body does not verify. Testing for one would also be the wrong judgement call. Slot keying is only ever established on a back-edge dependency, which makes `GetForEndIndex()` look like a valid stand-in -- but the synthetic prologue-prime and epilogue-drain pairs built by SyncEventIdAllocation::UpdateBackwardMatchSync inherit both `forEndIndex` and `eventIdNum` from the pair they derive from while deliberately carrying no slot, and they lower to static flags. Keying off the back edge would classify those as slot-keyed. They are created after all three current call sites, so this is a latent trap rather than a live defect, and the misclassification would be over-conservative rather than unsound -- but it is exactly the kind of thing a later reader fixes back. Name it for the property actually tested, aligned with the existing `slotSSAExpr` / `slotCount` vocabulary, and record in the doc comment why `slotSSAExpr` is the discriminator. No functional change: identical emitted sync on two_regions, two_regions_skewed and two_gets; all 27 multi_tile lit tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
/run a3 |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A3 板测完成（有跳过）
|
A3 板测失败
日志尾部 |
A5 板测成功
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
multi_tile_getresults consumed by one vector operation.Root cause
Legacy InsertSync selected the first viable producer/consumer slot expression when creating a multi-event back-edge sync. It then recorded that slot-keyed event as though the complete source pipe were synchronized. With two co-live dynamic slots, this could leave the second access unguarded.
Validation
PTOASPythonPackageagainst LLVM 21.1.8 in WSL with-j1(360/360 initial build; 6/6 final incremental build).multi_tile_lit tests: 26/26 passed.V -> MTE2wait, both TLOADs, TADD, then the matching static set; unambiguous single-slot cases still use dynamic event IDs.Closes #1118