Skip to content

Fix TPush CPU-SIM A5 support issue#109

Merged
Crystal-wzy merged 1 commit into
hw-native-sys:mainfrom
azizbek-khabibov:tpush
May 15, 2026
Merged

Fix TPush CPU-SIM A5 support issue#109
Crystal-wzy merged 1 commit into
hw-native-sys:mainfrom
azizbek-khabibov:tpush

Conversation

@azizbek-khabibov
Copy link
Copy Markdown

@azizbek-khabibov azizbek-khabibov commented May 4, 2026

To fix issue #101

Copy link
Copy Markdown

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

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 modifies the TPush implementation to correctly handle DIR_BOTH pipes by incorporating TileProd::Loc checks when determining the transfer direction. Feedback includes a potential bug where TileType::Mat transfers might be incorrectly recorded due to an incomplete check in the underlying IsC2VProducerTile function, along with a suggestion to improve code formatting and comment placement for better readability.

Comment thread include/pto/cpu/TPush.hpp
Comment thread include/pto/cpu/TPush.hpp Outdated
@azizbek-khabibov azizbek-khabibov force-pushed the tpush branch 2 times, most recently from e747033 to 127f262 Compare May 4, 2026 14:04
@azizbek-khabibov azizbek-khabibov changed the title Fix TPush CPU-SIM A5 support issue Fix TPush CPU-SIM A5 support problem May 5, 2026
@azizbek-khabibov azizbek-khabibov changed the title Fix TPush CPU-SIM A5 support problem Fix TPush CPU-SIM A5 support issue May 5, 2026
@azizbek-khabibov azizbek-khabibov force-pushed the tpush branch 2 times, most recently from a3f2a45 to 053f2bf Compare May 6, 2026 19:04
@zhoubot zhoubot mentioned this pull request May 8, 2026
@zhoubot
Copy link
Copy Markdown
Collaborator

zhoubot commented May 8, 2026

Triage review (2026-05-08): this is the focused PR in the TPUSH CPU-SIM cluster, and the patch applies to current main, but I would not merge it yet.

Issues to fix before review:

  • there are leftover commented-out implementation fragments and explanatory comments that look like debugging notes, e.g. the old SlotTile path and comments such as // popTileFromVecFiFoSplit;
  • the indentation change around auto *addr should be cleaned up;
  • the direction predicates need a short rationale/test matrix for DIR_C2V, DIR_V2C, and DIR_BOTH with TileType::Mat, Acc, and Vec, because this is exactly the area that caused [Bug] cpu/TPush.hpp sim does not correctly model a5 cross-core pipe semantics #101;
  • no CI checks are reported on the branch.

Please add or reference targeted CPU-SIM regression coverage for the failing #101 cases and run at least the relevant tpushpop/cross-core CPU tests. This should also be coordinated with #115 so only one canonical CPU-SIM fix remains open.

@UdacianND
Copy link
Copy Markdown

  • no CI checks are reported on the branch.

Hi. What is this CI check? How to run it?

@HaijianZ
Copy link
Copy Markdown
Contributor

1

@Crystal-wzy Crystal-wzy merged commit d779cd0 into hw-native-sys:main May 15, 2026
4 checks passed
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.

5 participants