Skip to content

Fix zero-loop sync propagation - #585

Closed
zhangstevenunity wants to merge 6 commits into
mainfrom
codex/fix-zero-loop-sync
Closed

Fix zero-loop sync propagation#585
zhangstevenunity wants to merge 6 commits into
mainfrom
codex/fix-zero-loop-sync

Conversation

@zhangstevenunity

Copy link
Copy Markdown
Collaborator

Summary

  • distinguish may-zero-trip loops from definitely-entered loops
  • keep conditional finder propagation for may-skip regions only
  • restore transitive alreadySync guard for conditional finders
  • update regression coverage for nested loop same-pipe pairing

Test

  • issue533_loop_zero_trip_sync_regression
  • issue564_k_loop_mte1_mte2_wait_regression
  • issue454_nested_loop_same_pipe_pair_regression
  • issue428_cube_sync_regression

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to track synchronization operations within potentially skipped code regions, such as zero-trip loops and if-statements without an else branch, to ensure correct synchronization state propagation. It adds a "mayZeroTrip" flag to loop elements and implements logic to mark sync entries as conditional. A review comment identifies an opportunity to optimize loops that are guaranteed to execute by fully propagating their synchronization state, which would prevent redundant synchronization operations.

Comment on lines +267 to +270
syncRecordList[bufferIdx].syncFinder =
syncRecordForList[bufferIdx].syncFinder;
syncRecordList[bufferIdx].syncFinderIsConditional =
syncRecordForList[bufferIdx].syncFinderIsConditional;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For loops that are definitely entered (mayZeroTrip is false), the synchronization state established within the loop body (including alreadySync) should be fully propagated to the outer context. The current implementation only copies syncFinder and syncFinderIsConditional, which may lead to redundant synchronization operations being inserted later because the transitive elimination logic won't see the alreadySync status from the body.

Suggested change
syncRecordList[bufferIdx].syncFinder =
syncRecordForList[bufferIdx].syncFinder;
syncRecordList[bufferIdx].syncFinderIsConditional =
syncRecordForList[bufferIdx].syncFinderIsConditional;
syncRecordList[bufferIdx] = syncRecordForList[bufferIdx];

@reedhecre

Copy link
Copy Markdown

Codex Review

该评论由 review 机器人自动更新。

  • PR: Fix zero-loop sync propagation #585 Fix zero-loop sync propagation
  • Author: zhangstevenunity
  • Base/Head: main / codex/fix-zero-loop-sync
  • Head SHA: 0b0e3c194c88
  • Trigger: 检测到新的 open PR
  • Generated At: 2026-04-26T05:35:12Z
  • Status: completed

Summary

未检查到 PR #585 存在问题

Findings

No issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants