Skip to content

docs: prefer ptoas autosync in user-facing samples#255

Open
zhoubot wants to merge 1 commit intozhangstevenunity:mainfrom
zhoubot:codex/ptoas-autosync-docs
Open

docs: prefer ptoas autosync in user-facing samples#255
zhoubot wants to merge 1 commit intozhangstevenunity:mainfrom
zhoubot:codex/ptoas-autosync-docs

Conversation

@zhoubot
Copy link
Contributor

@zhoubot zhoubot commented Mar 12, 2026

Summary

  • update PTOAS user-facing sync docs so PTODSL kernels default to autosync via ptoas --enable-insert-sync
  • remove manual event/wait usage from the main matmul sample that users copy first

Validation

  • python3 test/samples/MatMul/tmatmulk.py

Context

This keeps PTOAS aligned with the current PTODSL authoring contract: kernel source should stay explicit-sync-free and PTOAS should insert the needed sync during lowering.

Copy link

@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: 7768da7397

ℹ️ 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".

Comment on lines +21 to +24
# This sample intentionally contains NO explicit sync ops.
#
# Use it with:
# ptoas --enable-insert-sync

Choose a reason for hiding this comment

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

P1 Badge Restore sync safety for default MatMul sample builds

This update makes the sample depend on ptoas --enable-insert-sync, but first-party build/test paths still invoke ptoas without that flag (README.md lines 228-231, docker/Dockerfile lines 94-95 and 137-138, docker/test_ptoas_cli.sh lines 46-47). Because the CLI option defaults to off (tools/ptoas/ptoas.cpp lines 106-108), those existing flows now compile this sample with no explicit record_event/wait_event and no auto-inserted sync, which can leave TMOV/TMATMUL/TSTORE ordering unsafeguarded at runtime.

Useful? React with 👍 / 👎.

%sub = pto.subset %src[%i, %j] sizes [32, 32] : !pto.tile_buf<loc=vec, dtype=f16, rows=64, cols=64, v_row=64, v_col=64, blayout=row_major, slayout=none_box, fractal=512, pad=0>
```

##### `pto.set_validshape` - Rebind Tile Valid Row/Col

Choose a reason for hiding this comment

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

P2 Badge Remove unsupported pto.set_validshape from IR manual

The manual now documents pto.set_validshape as a concrete PTO IR op, but there is no corresponding op definition or binding in the repository (e.g., include/PTO/IR/PTOOps.td has no "set_validshape" op, and repo-wide search in include/, lib/, python/, and test/ finds no implementation). Publishing a non-existent op in the primary IR reference will cause users to write IR that current tooling cannot parse or lower.

Useful? React with 👍 / 👎.

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.

1 participant