Skip to content

docs: support GlobalTensor pipe entry~#602

Merged
zhangstevenunity merged 6 commits intomainfrom
codex/globaltensor-pipe-entry-docs
Apr 29, 2026
Merged

docs: support GlobalTensor pipe entry~#602
zhangstevenunity merged 6 commits intomainfrom
codex/globaltensor-pipe-entry-docs

Conversation

@zhangstevenunity
Copy link
Copy Markdown
Collaborator

No description provided.

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 updates the PTO IR manual and design documentation to support global entry in pipe communication, allowing for GlobalTensor-like GM view descriptors alongside existing local tile buffers. It introduces new talloc operations for producer-side allocation and modifies initialization procedures for global-only GM FIFO pipes to omit local consumer buffers. Feedback was provided to ensure consistency in the documentation regarding the binding of talloc operations.

Comment thread docs/PTO_IR_manual.md
`pto.aic_initialize_pipe` / `pto.aiv_initialize_pipe` with the matching
`pto.tpush_*` / `pto.tpop_*` / `pto.tfree_*` ops in the same function.
- `slot_size` is expressed in bytes and uses the pre-split logical tile size.
`pto.talloc_*` / `pto.tpush_*` / `pto.tpop_*` / `pto.tfree_*` ops in the same
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The updated description mentions pto.talloc_* but the previous line 7901 only lists pto.talloc_* as an example. It is better to be consistent in the documentation regarding whether talloc is part of the binding or just an example.

@reedhecre
Copy link
Copy Markdown

reedhecre commented Apr 28, 2026

Codex Review

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

  • PR: docs: support GlobalTensor pipe entry~ #602 docs: support GlobalTensor pipe entry~
  • Author: zhangstevenunity
  • Base/Head: main / codex/globaltensor-pipe-entry-docs
  • Head SHA: 3c8857a25cc7
  • Trigger: PR 有新提交
  • Generated At: 2026-04-29T06:16:27Z
  • Previous Head SHA: 1bb54aba5af5
  • Status: completed

Summary

检查到 3 个问题:1 个会直接让 EmitC 输出代码编译失败,2 个会破坏 GlobalTensor pipe 的跨函数配对/协议一致性。

Findings

  1. P1 GlobalTensor pipe init 会生成未定义的 PTOAS__GLOBAL_TENSOR_DATA 调用 lib/PTO/Transforms/PTOToEmitC.cpp:11027

materializeTensorViewDataPointer() 会把 !pto.tensor_view 形式的 gm_addr 统一降成 PTOAS__GLOBAL_TENSOR_DATA(...) 调用,但 helper 只在模块里仍然存在 pto.partition_view 时才会被注入。这样一来,纯 GlobalTensor pipe 且没有 partition_view 的用例(例如这次新增的 tpush_tpop_globaltensor_internal_a3.pto 路径)会生成引用该 helper 的 C++,却没有对应定义,最终导致 EmitC 输出代码无法编译。

  1. P2 Global-only pipe 的 peer 匹配键不稳定,既会误合并也会漏配对 lib/PTO/Transforms/PTOResolveReservedBuffersPass.cpp:165

getGlobalTensorPipeKey() 对 global-only pipe 的配对键只用了 __pto.frontend_id + dir_mask,没有把函数对关系编码进去;而没有这个 attr 的内部 IR 又退化成用 op 地址做键。前者会把不同 kernel pair 里恰好都用 id = 0 的 pipe 错误合并,进而在 requires peer pipe init ops to agree on direction and pipe shape 这里误报;后者则会让直接写 internal initialize_l2g2l_pipe 的 producer / consumer 永远配不上对,自动分配出不同的 flag_base。这两种情况都会破坏合法的多函数 GlobalTensor pipe 场景。

  1. P2 Global-only pipe 缺少跨函数的 split/nosplit 一致性校验 lib/PTO/Transforms/PTOInferValidatePipeInitPass.cpp:244

PTOInferValidatePipeInitPass 只通过 local_addr / peer_local_addr 建立 peer 关系,而 global-only pipe 恰好没有这两个操作数,所以两端会各自独立推断 nosplit。结果是同一个 frontend pipe 上,producer 侧用 split = 0、consumer 侧用 split = 1 也能一起通过;生成的两端 TPipe 模板参数和同步协议会不一致,运行时很容易卡死或读错槽位。

@zhangstevenunity zhangstevenunity marked this pull request as ready for review April 29, 2026 02:14
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23447a4d8b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

id = std::to_string(idAttr.getInt());
else
id = std::to_string(reinterpret_cast<uintptr_t>(info.op));
return PipePeerKey{"__pto_globaltensor_pipe", "id_" + id, info.dirMask};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Include function scope in global-only pipe grouping key

getGlobalTensorPipeKey collapses every global-only pipe with the same frontend id and dirMask into one module-wide component, because it uses a constant owner ("__pto_globaltensor_pipe"). In collectPeerAwareInit this causes unrelated kernels that reuse the default id = 0 to be merged, and later buildPeerAwareComponents enforces a single shared signature/flag_base for all of them. A valid module with multiple independent pipe pairs can then fail with a false "peer pipe init ops must agree" error (or get unintended shared flag allocation) even though IDs are only required to be unique per function.

Useful? React with 👍 / 👎.

@zhangstevenunity zhangstevenunity merged commit 2a06fed into main Apr 29, 2026
14 checks passed
@reedhecre
Copy link
Copy Markdown

A5 板测成功

  • 触发方式:merged
  • 源码提交:2a06fed82bfa
  • 结果汇总:OK 17 / FAIL 0 / SKIP 0
  • 日志:/root/ptoas-board-monitor-a5/logs/20260429_170906_merged_pr602.log
  • 结果 TSV:/root/ptoas-board-monitor-a5/logs/20260429_170906_merged_pr602.tsv

@reedhecre
Copy link
Copy Markdown

A3 板测完成(有跳过)

  • 触发方式:merged
  • 源码提交:2a06fed82bfa
  • 结果汇总:OK 202 / FAIL 0 / SKIP 1
  • 日志:/home/zhongxuan/ptoas-board-monitor/runtime/logs/20260429_170905_merged_pr602.log
  • 结果 TSV:/home/zhongxuan/ptoas-board-monitor/runtime/logs/20260429_170905_merged_pr602.tsv

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