[A5][Sync] remove identity tmov before insert-sync#420
Open
TaoTao-real wants to merge 16 commits intohw-native-sys:mainfrom
Open
[A5][Sync] remove identity tmov before insert-sync#420TaoTao-real wants to merge 16 commits intohw-native-sys:mainfrom
TaoTao-real wants to merge 16 commits intohw-native-sys:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new optimization pass, PTORemoveIdentityTMovPass, which removes identity pto.tmov operations where the source and destination are the same SSA value. This pass is specifically gated for the A5 architecture and is integrated into the ptoas tool to run before pto-insert-sync to prevent unnecessary synchronization edges. The review feedback suggests optimizing the pass implementation by performing the erasure directly within the walk callback, eliminating the need for an intermediate SmallVector and a secondary loop.
Comment on lines
+56
to
+67
| SmallVector<TMovOp> toErase; | ||
| funcOp.walk([&](TMovOp op) { | ||
| if (canEraseIdentityTMov(op)) | ||
| toErase.push_back(op); | ||
| }); | ||
|
|
||
| for (TMovOp op : toErase) { | ||
| Value result = op.getResult(); | ||
| if (result && !result.use_empty()) | ||
| result.replaceAllUsesWith(op.getDst()); | ||
| op.erase(); | ||
| } |
There was a problem hiding this comment.
The current implementation first collects all TMovOps to be erased into a SmallVector and then iterates over this vector to perform the erasure. This two-step process can be simplified and made more efficient. You can perform the erasure directly within the walk callback. Since TMovOp has no regions, it's safe to erase it during the walk, which avoids the need for intermediate storage and a second loop.
funcOp.walk([&](TMovOp op) {
if (canEraseIdentityTMov(op)) {
Value result = op.getResult();
if (result && !result.use_empty()) {
result.replaceAllUsesWith(op.getDst());
}
op.erase();
}
});
Contributor
Author
|
/run A5 identity_tmov_autosync_a5_only |
A5 板测成功
|
Collaborator
|
/run a5 test/basic/identity_tmov_autosync_a5_only.pto |
A5 板测成功
|
Contributor
Author
|
/run A5 identity_tmov_if_else_alias_a5 |
A5 板测失败
日志尾部 |
Contributor
Author
|
/run A5 identity_tmov_if_else_alias_a5 --pto-level=level3 |
A5 板测成功
|
Contributor
Author
|
/run A5 issue828_softmax_rescale_incore_1_a5 --pto-level=level3 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #420issue828_softmax_rescale_incore_1_a5
|
Contributor
Author
|
/run A5 issue828_softmax_rescale_incore_1_a5 --pto-level=level3 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #420issue828_softmax_rescale_incore_1_a5
|
Contributor
Author
|
/run A5 identity_tmov_if_else_alias_a5 --pto-level=level3 |
A5 板测成功
|
Contributor
Author
|
/run A5 issue828_softmax_rescale_incore_1_a5 --pto-level=level3 |
A5 板测成功
|
Contributor
Author
|
/run A5 issue828_softmax_rescale_incore_1_a5 --pto-level=level3 |
A5 板测失败详情：PR #420issue828_diag_else_3tmov_only_a5
|
A5 板测成功
|
Contributor
Author
|
/run A5 issue828_diag_if_identity_only_a5 --pto-level=level3 --disable-identity-tmov-cleanup |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #420issue828_diag_if_identity_only_a5
|
Contributor
Author
|
/run a5 issue828_softmax_rescale_incore_1_a5_if_aligned issue828_softmax_rescale_incore_1_a5_else_aligned --pto-level=level3 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #420issue828_softmax_rescale_incore_1_a5_else_aligned
|
Contributor
Author
|
/run A5 issue828_diag_else_trace_print_a5 --pto-level=level3 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #420issue828_diag_else_trace_print_a5
|
Contributor
Author
|
/run A5 issue828_diag_else_trace_print_a5 --pto-level=level3 |
A5 板测成功
|
Contributor
Author
|
/run A5 issue828_diag_else_3tmov_only_a5 --pto-level=level3 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #420issue828_diag_else_3tmov_only_a5
|
Contributor
Author
|
/run a5 issue828_diag_else_3tmov_only_a5 issue828_diag_else_3tmov_vrow1_a5 issue828_diag_else_3tmov_rowmajor_a5 --pto-level=level3 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #420issue828_diag_else_3tmov_only_a5
|
Contributor
Author
|
/run a5 issue828_diag_else_3tmov_vrow1_a5 --pto-level=level3 |
A5 板测成功
|
Contributor
Author
|
/run a5 issue828_diag_else_3tmov_rowmajor_a5 --pto-level=level3 |
A5 板测成功
|
Contributor
Author
|
/run a5 issue828_diag_else_3tmov_vrow1_a5 --pto-level=level3 |
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
4 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
pto-remove-identity-tmov) that erasespto.tmov ins(%x) outs(%x)before auto-sync runs.ptoasdirectly beforePTOInsertSyncwhen--enable-insert-syncis enabled.test/basic/identity_tmov_autosync_a5_only.pto.Motivation
tmovbeing treated as a real producer/consumer by auto-sync, which can create spurious sync edges around a hardware no-op move.Design
PTORemoveIdentityTMovPass(func pass).pto.target_arch == "a5".tmov(src == dstSSA value).dst, rewires uses todstbefore erase.PTOResolveReservedBuffers -> PTORemoveIdentityTMov -> PTOInsertSync.Testing
ninja -C /Users/lishengtao/Documents/PTO/_codex_worktrees/ptoas_identity_tmov_a5/build ptoasptoas --pto-arch=a5 --enable-insert-sync test/basic/identity_tmov_autosync_a5_only.pto | FileCheck ... --check-prefix=A5ptoas --pto-arch=a3 --enable-insert-sync test/basic/identity_tmov_autosync_a5_only.pto | FileCheck ... --check-prefix=A3ptoas --pto-arch=a5 --enable-insert-sync test/basic/tmov_acc_mat_pipe_selection.pto | FileCheck ...Risk / Rollback