feat(codegen): validate shift op family - #2189
Draft
hashiqiqixian wants to merge 4 commits into
Draft
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughShift operations now enforce matching tile contracts, bounded scalar counts, signed same-width scalar encoding, and row-major layouts. New unit and runtime tests cover supported dtypes, valid regions, platforms, code generation, and hardware-specific constraints. English and Chinese documentation reflect the updated mappings and status. ChangesTile shift contracts and runtime coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ShiftCase
participant PyPTOProgram
participant TileShiftOps
participant TestRunner
ShiftCase->>PyPTOProgram: build shift program
PyPTOProgram->>TileShiftOps: apply tile or scalar shift
TileShiftOps->>TestRunner: write valid output region
TestRunner->>ShiftCase: compare expected values
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d27d094fd1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
hashiqiqixian
force-pushed
the
feat/ptoas-b08-shift
branch
from
July 29, 2026 17:21
a58f7f6 to
55158b2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/en/dev/ptoas-op-status.md`:
- Around line 277-278: Update the same-name ST coverage breakdown from 48
regular ops to 50 in both docs/en/dev/ptoas-op-status.md (lines 277-278) and
docs/zh-cn/dev/ptoas-op-status.md (lines 263-264), preserving the distributed
count and totals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 317a02d9-044b-48c8-a020-c5d01ce14e11
📒 Files selected for processing (11)
docs/en/dev/codegen/00-pto_codegen.mddocs/en/dev/ptoas-op-status.mddocs/zh-cn/dev/codegen/00-pto_codegen.mddocs/zh-cn/dev/ptoas-op-status.mdpython/pypto/ir/op/tile_ops.pypython/pypto/language/op/tile_ops.pysrc/backend/common/pto_ops_elementwise.cppsrc/ir/op/tile_ops/elementwise.cpptests/st/runtime/ops/test_shift.pytests/ut/codegen/test_pto_codegen.pytests/ut/ir/operators/test_tile_ops.py
Comment on lines
+277
to
+278
| 112 have same-name ST coverage (108 regular STs and 4 distributed STs); 60 lack same-name ST coverage | ||
| (48 regular and 10 distributed); within these 204, another 32 ops are not suitable for standalone STs. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files mentioning ptoas-op-status:"
git ls-files | rg '(^|/)ptoas-op-status\.md$|ptoas-op-status'
echo
echo "English stats section around relevant lines:"
if [ -f docs/en/dev/ptoas-op-status.md ]; then
nl -ba docs/en/dev/ptoas-op-status.md | sed -n '240,290p'
fi
echo
echo "Chinese stats section around relevant lines:"
if [ -f docs/zh-cn/dev/ptoas-op-status.md ]; then
nl -ba docs/zh-cn/dev/ptoas-op-status.md | sed -n '230,280p'
fi
echo
echo "Diff stats around relevant hunks:"
git diff -- docs/en/dev/ptoas-op-status.md docs/zh-cn/dev/ptoas-op-status.md | sed -n '1,220p'
echo
echo "Other nearby totals/count mentions in English docs:"
rg -n "112|60|32|204|48|108|4|10" docs/en/dev/ptoas-op-status.md
python3 - <<'PY'
import re
from pathlib import Path
for path in ["docs/en/dev/ptoas-op-status.md", "docs/zh-cn/dev/ptoas-op-status.md"]:
text = Path(path).read_text()
# Extract the exact reported phrase if present.
m = re.search(r"\(48 regular and 10 distributed\)", text)
print(f"{path}: phrase found={bool(m)}")
# Do not read files as executable; arithmetic check from reported numbers:
stated = 60
breakdown = 48 + 10
print(f"{path}: stated_lacking={stated}, breakdown_sum={breakdown}, equal={stated==breakdown}, within_204_consistent={112+stated+32==204}")
PYRepository: hw-native-sys/pypto
Length of output: 348
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)ptoas-op-status\.md$|ptoas-op-status'
echo
echo "English section around relevant lines:"
sed -n '240,290p' docs/en/dev/ptoas-op-status.md
echo
echo "Chinese section around relevant lines:"
sed -n '230,280p' docs/zh-cn/dev/ptoas-op-status.md
echo
echo "Diff around relevant hunks:"
git diff -- docs/en/dev/ptoas-op-status.md docs/zh-cn/dev/ptoas-op-status.md | sed -n '1,220p'
echo
echo "Number mentions in affected file:"
rg -n "112|60|32|204|48|108|4|10" docs/en/dev/ptoas-op-status.md
python3 - <<'PY'
from pathlib import Path
for path in ["docs/en/dev/ptoas-op-status.md", "docs/zh-cn/dev/ptoas-op-status.md"]:
text = Path(path).read_text()
phrase = "(48 regular and 10 distributed)"
print(f"{path}: phrase found={phrase in text}")
print(f"{path}: stated_lacking=60, breakdown_sum=48+10={48+10}, equal={60==48+10}, within_204_consistent={112+60+32==204}")
PYRepository: hw-native-sys/pypto
Length of output: 9708
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find PTOAS/PTOOps source files:"
git ls-files | rg '(^|/)(PTOAS|PTOOps|ptoas|ptoas-op-status|ptoas_).*' || true
echo
echo "Search for same-name ST/distributed ST/PTOAS counts in tracked files:"
rg -n "same-name|same name|same-name ST|no same-name|缺少同名|未包含同名|same name PTO|same-name PTO|113|112|75|60|58|50|48|108|10|4|32|204" -S . \
-g '!node_modules' -g '!build' -g '!dist' -g '!*.svg' -g '!*.png' \
| head -n 200
echo
echo "Git diff stat:"
git diff --statRepository: hw-native-sys/pypto
Length of output: 20937
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
from collections import Counter
# Read only the docs as data to classify same-name ST coverage flags without executing repo code.
def parse_status(table_text, defs):
status_re = re.compile(r'<!--\s*def:\s*([^>]+)-->')
rows = []
ops = []
for line in table_text.splitlines():
m = status_re.search(line)
if m:
defs[m.group(1)] = line
if line.startswith('|'):
cells = re.split(r'\s*\|\s*', line.strip())
ops.append(cells[1])
rows.append([c.strip() for c in cells])
return rows, ops
lang = 'en' # English table should match; Chinese rows should have same op ordering.
text = Path('docs/en/dev/ptoas-op-status.md').read_text('utf-8')
# Extract table between first "|" and Stats paragraph.
start = text.index('|', text.index('/ ptoas-op-status.md', text.find('`')))
end = text.index('**Stats**')
table_text = text[start:end]
rows, ops = parse_status(table_text, {})
# Count per-category/section by counting non-header rows with an op name.
# Column definitions in comments are not numeric fields, but the status columns after section rows are simple ✅/❌/— markers.
def same_name_status(cell):
# Column after op/type/category/status/status/status/status/status is first same-name status.
# Use status def mapping; not easy here without definitions, so scan for ✅ marker in likely status columns.
return cell in ('✅', '❌', '—')
# Instead, derive counts from the rendered commentary/status flags by simple line scan for missing same-name comment.
missing = []
for j, op in enumerate(ops):
row = rows[j]
# Use the full original line containing this op for parsing when available.
line = ''
# no-op
pass
# Lightweight aggregate from same-name status columns using known comment pattern is too brittle.
# Use source data from rows if parsed; only available columns so far are limited by this function.
PY
echo "Manual row counts for same-name ST columns parsed from markdown:"
python3 - <<'PY'
import re
from pathlib import Path
lang = 'en'
text = Path('docs/en/dev/ptoas-op-status.md').read_text('utf-8')
table = text[text.index('<!-- markdownlint-disable'):text.index('**Stats**')]
def col_index(cell):
for m in re.finditer(r'<!-- (?:header|def):\s*([^>]+)', table):
pass
# Actually extract header comment line count by scanning first rows with comment patterns.
return None
PY
sed -n '150,285p' docs/en/dev/ptoas-op-status.md | awk -F'|' '{
name=$2; gsub(/^ +| +$/,"",name);
if (name !~ /^ *[|A-Za-z_-]+/) next
if (NF < 9) next
# Columns of interest after fixed columns: likely same-name status columns are consistent in rows containing ops.
n++
# Print row and first 9 cells for manual/verified tally.
# print n, name, $4, $5
}'Repository: hw-native-sys/pypto
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Exact lines:"
sed -n '272,282p' docs/en/dev/ptoas-op-status.md
sed -n '258,268p' docs/zh-cn/dev/ptoas-op-status.md
echo
echo "Parse exact same-name ST columns from English table:"
python3 --version
python3 - <<'PY'
from pathlib import Path
p = Path('docs/en/dev/ptoas-op-status.md')
text = p.read_text('utf-8')
# Use exact line numbers from previous scan to avoid substring assumptions.
lines = text.splitlines()
for lineno in range(276, 282):
print(f"L{lineno}: {lines[lineno-1]}")
# Extract from the raw lines around the op rows through Stats, parsing only markdown cells.
table_start = lines.index('| pto.comm.tassign |') if any('pto.comm.tassign' in line for line in lines) else 0
table_end = next(i for i,l in enumerate(lines) if 'Stats' in l)
rows = []
header = None
for i,l in enumerate(lines[table_start:table_end], start=table_start):
cells = [c.strip() for c in l.split('|', 73)[1:-1]]
if not header:
# Header is the first row without a real op name.
if all(c and not c.startswith('pto.') and not c.startswith('PTO') for c in cells[:7]):
header = cells
rows.append(('header', cells))
continue
if not all(c and not c.startswith('pto.') and not c.startswith('PTO') for c in cells[:7]):
continue
rows.append((i, cells))
header = rows[0][1]
print('HEADER:')
print({idx: h for idx, h in header.items()})
counts = {'same_no': 0, 'same_yes': 0, 'dist': 0}
for idx, row in rows[1:]:
name = row[1]
# Find column names and values by header index.
print(f"\n{idx}: {name}")
for col in header:
if 'same-name' in col.lower() or 'ST✅' in col or 'ST status' in col.lower():
print(col, row[:73].get(col))
val = row[:73].get(col)
if val is not None and val.strip():
counts['same_no'] += 1
# The 8th cell is likely duplicate/stability evidence? Instead parse column names.
# Summarize only from cells with same-name-related columns if parsed, then print rows summary.
PYRepository: hw-native-sys/pypto
Length of output: 1640
Fix the “lacking same-name ST” breakdown in both language versions. The paragraph states 60 ops lack same-name coverage, but the regular distributed breakdown is 48 + 10 = 58. Since the totals cross-check as 112 + 60 + 32 = 204, update the regular lacking count from 48 to 50 in both docs.
docs/en/dev/ptoas-op-status.md#L277-L278docs/zh-cn/dev/ptoas-op-status.md#L263-L264
📍 Affects 2 files
docs/en/dev/ptoas-op-status.md#L277-L278(this comment)docs/zh-cn/dev/ptoas-op-status.md#L263-L264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/en/dev/ptoas-op-status.md` around lines 277 - 278, Update the same-name
ST coverage breakdown from 48 regular ops to 50 in both
docs/en/dev/ptoas-op-status.md (lines 277-278) and
docs/zh-cn/dev/ptoas-op-status.md (lines 263-264), preserving the distributed
count and totals.
Contributor
|
按 #2166 复核当前 head
按 #2166 的 G3/G5，请先把这两行改回 |
hashiqiqixian
force-pushed
the
feat/ptoas-b08-shift
branch
from
July 30, 2026 02:00
55158b2 to
3cab369
Compare
Contributor
Author
|
Closing this later PTOAS batch for now so work can proceed serially from B02 and B03. The branch is preserved for reopening when its turn arrives. |
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
2 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
tile.shl,tile.shr,tile.shls, andtile.shrswith the current PTO-ISA dtype, valid-region, scalar, and row-major layout contractsValidation
git diff --check: passed/data/chenshenai/test2: passed.ptofiles contain exactpto.tshl,pto.tshr,pto.tshls, andpto.tshrsoperand types and orderRuntime coverage includes left/right shifts, signed/unsigned 16-bit tile forms, zero/one/width-minus-one counts, scalar/tile forms, and narrowed
valid_shape. A5 hardware validation remains pending and is documented in the status matrix.