feat: add B11 indexed concat and exact fillpad - #2192
Conversation
📝 WalkthroughWalkthroughAdds ChangesTile operations and PTO lowering
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 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 |
910182a to
7cfe9e2
Compare
|
按 #2166 复核当前 head
|
7cfe9e2 to
b37d297
Compare
|
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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`:
- Line 194: Revert the TFILLPAD_INPLACE exec/ST status from ✅ to ❌ and revise
its note to state that only the confirmed A2 zero/max/min cases are validated
pending broader on-device matrix coverage. Apply the identical update in
docs/en/dev/ptoas-op-status.md:194 and docs/zh/dev/ptoas-op-status.md:180 to
keep both localized status tables synchronized.
In `@src/backend/common/pto_ops_elementwise.cpp`:
- Around line 147-155: Strengthen
test_pto_codegen_concat_idx_uses_four_inputs_and_reused_destination() to assert
the exact pto.tconcatidx codegen contract: verify all four inputs appear in
order, each has the expected operand type annotation, and outs contains only dst
with the destination tile/buffer type. Keep the existing opcode and ins/outs
marker checks.
- Around line 511-512: Update the tile.concat_idx registration to attach the
BackendTileLayoutSpec row-max constraints for input operands 0..3 and the result
tile, while preserving the existing MakeConcatIdxCodegenPTO callback.
In `@src/ir/op/tile_ops/transform.cpp`:
- Around line 949-964: In the tile.concat_idx validation block, remove the
physical row-count loop that compares type->shape_[0] with dst->shape_[0].
Retain validation using GetValidShape for matching valid row counts, and update
the idx0_valid/idx1_valid column validation to require each valid column extent
to be at least one rather than exactly equal to one.
- Around line 971-985: Update the tile.concat_idx registration to add row-major
set_input_layout constraints for inputs 0 through 4 and a row-major
set_output_layout constraint for the result. Keep the existing memory-space and
output-reuse settings unchanged.
- Around line 938-945: Update the tile.concat_idx validation around
is_index_dtype and the idx0/idx1 checks to reject UINT8, UINT16, and UINT32
index tiles before PTO emission; accept only matching signed INT8, INT16, or
INT32 index dtypes while preserving the existing source-data validation.
In `@tests/ut/codegen/test_pto_codegen.py`:
- Around line 498-523: Strengthen
test_pto_codegen_concat_idx_uses_four_inputs_and_reused_destination to validate
the emitted pto.tconcatidx contract: assert ins() contains exactly the four
source/index operands, outs() contains exactly one operand, and that operand
matches the SSA value loaded into dst and passed as the reused
destination/result target. Replace the current presence-only assertions while
preserving the existing program setup.
In `@tests/ut/ir/operators/test_concat_idx.py`:
- Around line 41-64: Update test_concat_idx_returns_destination_type to match
the PTOAS index-type contract: remove UINT8, UINT16, and UINT32 from the
successful index_dtype parameterization, or change those cases to assert the IR
op’s documented rejection/coercion behavior. Keep signed index dtype coverage
and destination type assertions unchanged.
- Around line 67-77: Update test_concat_idx_rejects_invalid_index_contract so an
index tensor with valid_cols == 2, represented by bad_cols, is accepted rather
than expected to raise. Remove the columns-equal-to-1 failure assertion and
verify concat_idx succeeds with the existing matching-row inputs, while
retaining the invalid integer-index assertion.
🪄 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: 0796b0fb-e939-4787-a0a9-cb80f72a1707
📒 Files selected for processing (14)
docs/en/dev/codegen/00-pto_codegen.mddocs/en/dev/ptoas-op-status.mddocs/zh/dev/codegen/00-pto_codegen.mddocs/zh/dev/ptoas-op-status.mdpython/pypto/ir/op/tile_ops.pypython/pypto/language/__init__.pypython/pypto/language/op/__init__.pypython/pypto/language/op/tile_ops.pysrc/backend/common/pto_ops_elementwise.cppsrc/ir/op/tile_ops/transform.cpptests/st/runtime/ops/test_concat_idx.pytests/st/runtime/ops/test_fillpad_inplace.pytests/ut/codegen/test_pto_codegen.pytests/ut/ir/operators/test_concat_idx.py
| | pto.tfillpad | TFILLPAD | tile+tensor | ✅ | ✅ | ✅ | ✅ | — | | | ||
| | pto.tfillpad_expand | TFILLPAD_EXPAND | tile+tensor | ✅ | ✅ | ✅ | ✅ | — | | | ||
| | pto.tfillpad_inplace | TFILLPAD_INPLACE | tile | ✅ | ✅ | ❌ | ❌ | — | the current codegen emits `pto.tfillpad` | | ||
| | pto.tfillpad_inplace | TFILLPAD_INPLACE | tile | ✅ | ✅ | ❌ | ✅ | — | corrected to exact same-name emission; zero/max/min modes pass on A2/A3 | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
TFILLPAD_INPLACE marked ST-supported in both English and Chinese status docs, but hardware evidence is narrower than the claimed status. Per PR objectives, only three older A2 zero/max/min cases have actual hardware confirmation, while the new test suite defines a much broader dtype × mode × valid-shape × platform matrix that hasn't itself been run to conclusion on hardware. The shared root cause is one overclaiming status value duplicated across the two localized docs.
docs/en/dev/ptoas-op-status.md#L194: revert the exec/ST column to ❌ and adjust the note until the broader matrix is validated on-device.docs/zh/dev/ptoas-op-status.md#L180: apply the identical reversion to keep the Chinese doc in sync with the English one.
📍 Affects 2 files
docs/en/dev/ptoas-op-status.md#L194-L194(this comment)docs/zh/dev/ptoas-op-status.md#L180-L180
🤖 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` at line 194, Revert the TFILLPAD_INPLACE
exec/ST status from ✅ to ❌ and revise its note to state that only the confirmed
A2 zero/max/min cases are validated pending broader on-device matrix coverage.
Apply the identical update in docs/en/dev/ptoas-op-status.md:194 and
docs/zh/dev/ptoas-op-status.md:180 to keep both localized status tables
synchronized.
| static std::string MakeConcatIdxCodegenPTO(const CallPtr& op, codegen::CodegenBase& codegen_base) { | ||
| auto& codegen = AsPto(codegen_base); | ||
| CheckArity(op, "pto.tconcatidx", 5); | ||
| std::vector<std::pair<std::string, std::string>> inputs; | ||
| inputs.reserve(4); | ||
| for (size_t i = 0; i < 4; ++i) { | ||
| inputs.emplace_back(codegen.GetExprAsCode(op->args_[i]), codegen.GetExprTypeAnnotation(op->args_[i])); | ||
| } | ||
| EmitInsOuts(codegen, "pto.tconcatidx", inputs); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files around pto_ops_elementwise =="
fd -a 'pto_ops_elementwise\.cpp|codegen.*test|test.*codegen|.*ptos?.*test.*' . | sed 's#^\./##' | head -100
echo
echo "== target file outline =="
ast-grep outline src/backend/common/pto_ops_elementwise.cpp --view expanded || true
echo
echo "== target lines 100-190 =="
sed -n '100,190p' src/backend/common/pto_ops_elementwise.cpp | nl -ba -v100
echo
echo "== search concatidx/tconcatidx/codegen test markers =="
rg -n "tconcatidx|concatidx|tconcat|ins\(|outs\(|dst|destination|dest" src test tests .github tools bench 2>/dev/null | head -200 || trueRepository: hw-native-sys/pypto
Length of output: 3464
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file outline =="
ast-grep outline src/backend/common/pto_ops_elementwise.cpp --view expanded || true
echo
echo "== target lines 100-190 =="
nl -ba src/backend/common/pto_ops_elementwise.cpp | sed -n '100,190p'
echo
echo "== search concatidx/tconcatidx/codegen test markers =="
rg -n "tconcatidx|concatidx|tconcat|pto\.(tconcatidx|fillpad_inplace)|ins\(|outs\(|dst|destination|dest" src test tests .github tools bench 2>/dev/null | head -300 || true
echo
echo "== likely pto/ptotest files =="
git ls-files | rg 'ptotest|pto.*test|test.*pte|code.*test|elementwise.*pt|common/.*test' | head -200Repository: hw-native-sys/pypto
Length of output: 1918
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file lines 100-190 =="
awk 'NR>=100 && NR<=190 { printf "%6d %s\n", NR, $0 }' src/backend/common/pto_ops_elementwise.cpp
echo
echo "== registration/ops around concat_idx =="
awk 'NR>=420 && NR<=510 { printf "%6d %s\n", NR, $0 }' src/backend/common/pto_ops_elementwise.cpp
echo
echo "== test_pto references to concat_idx/tconcatidx/source =="
python3 - <<'PY'
import pathlib, re
for p in pathlib.Path('tests').rglob('*'):
if p.is_file():
try:
txt=p.read_text(errors='ignore')
except Exception:
continue
if 'concat_idx' in txt or 'tconcatidx' in txt:
print(f'\n--- {p} ---')
for i,line in enumerate(txt.splitlines(),1):
if 'concat_idx' in line or 'tconcatidx' in line:
print(f'{i}: {line}')
PY
echo
echo "== all files with concat_idx or tconcatidx =="
python3 - <<'PY'
import pathlib
for p in pathlib.Path('.').rglob('*'):
if not p.is_file(): continue
if any(part in {'.git','build','dist'} for part in p.parts): continue
try:
txt = p.read_text(errors='ignore')
except Exception:
continue
if 'concat_idx' in txt or 'tconcatidx' in txt or 'pto.tconcatidx' in txt:
print(p)
PY
echo
echo "== inspect EmitInsOuts definitions/usages =="
rg -n "EmitInsOuts|MakeConcatIdxCodegenPTO|ptotest|ptotest|pto.*test" src backend tests -g '*.cpp' -g '*.py' -g '*.cc' -g '*.h' | head -300 || trueRepository: hw-native-sys/pypto
Length of output: 19862
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== test_pto_codegen concat_idx test =="
awk 'NR>=480 && NR<=535 { printf "%6d %s\n", NR, $0 }' tests/ut/codegen/test_pto_codegen.py
echo
echo "== EmitInsOuts implementation =="
awk 'NR>=350 && NR<=380 { printf "%6d %s\n", NR, $0 }' src/backend/common/pto_ops_shared.cpp
awk 'NR>=70 && NR<=90 { printf "%6d %s\n", NR, $0 }' src/backend/common/pto_ops_internal.h
echo
echo "== concat_idx registration and op definitions =="
awk 'NR>=505 && NR<=520 { printf "%6d %s\n", NR, $0 }' src/backend/common/pto_ops_elementwise.cpp
rg -n -C 5 "tile\.concat_idx|concat_idx|set_output_reuses_input|set_input_layout|set_output_layout" python src tests/ut/ir/operators/test_concat_idx.py tests/st/runtime/ops/test_concat_idx.py | head -240Repository: hw-native-sys/pypto
Length of output: 27955
Assert the exact tconcatidx codegen contract.
test_pto_codegen_concat_idx_uses_four_inputs_and_reused_destination() only checks pto.tconcatidx plus ins(/outs( markers. Add assertions for the four inputs in order, the operand type annotations, and that outs contains only dst with the destination tile/buffer type.
🤖 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 `@src/backend/common/pto_ops_elementwise.cpp` around lines 147 - 155,
Strengthen test_pto_codegen_concat_idx_uses_four_inputs_and_reused_destination()
to assert the exact pto.tconcatidx codegen contract: verify all four inputs
appear in order, each has the expected operand type annotation, and outs
contains only dst with the destination tile/buffer type. Keep the existing
opcode and ins/outs marker checks.
| reg("tile.concat_idx", | ||
| [](const CallPtr& op, codegen::CodegenBase& codegen) { return MakeConcatIdxCodegenPTO(op, codegen); }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files =="
git ls-files | rg 'src/backend/common/pto_ops_elementwise\.cpp|src/backend|pto_ops_elementwise' | head -100
echo "== target outline =="
ast-grep outline src/backend/common/pto_ops_elementwise.cpp --view expanded || true
echo "== target lines =="
sed -n '1,90p; 450,560p' src/backend/common/pto_ops_elementwise.cpp
echo "== searches for concat_idx / layout setup / MakeConcatIdxCodegenPTO =="
rg -n "MakeConcatIdxCodegenPTO|concat_idx|row.*major|col.*major|LayoutConfig|layout" src/backend/common/pto_ops_elementwise.cpp src/backend -S | head -200
echo "== relevant files list =="
git ls-files src/backend | head -200Repository: hw-native-sys/pypto
Length of output: 29275
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pto_ops_elementwise concat_idx implementation =="
sed -n '120,210p; 488,515p' src/backend/common/pto_ops_elementwise.cpp
echo "== pto_ops_common subview layout handling =="
sed -n '1,220p; 380,475p' src/backend/common/pto_ops_common.cpp
echo "== pto layout codegen for tile_type =="
rg -n "pto\.tile_type|LayoutConfig|LayoutSpec|TileLayoutSpec|input_layouts|output_layout" src backend include -S | head -240
echo "== backend registry entry layout spec behavior =="
sed -n '400,505p' src/backend/common/backend.cpp
echo "== call stack or type-check for MakeConcatIdxCodegenPTO inputs =="
grep -n "MakeConcatIdxCodegenPTO\|tile\.concat_idx\|concat_idx" src -R -S
echo "== inspect backend registry header if present =="
fd -i 'backend.*\.h' src | head -20
sed -n '1,220p' src/backend/common/backend_registry.cpp 2>/dev/null || true
sed -n '1,220p' src/backend/common/backend_registry.h 2>/dev/null || trueRepository: hw-native-sys/pypto
Length of output: 13174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== resolve_backend_op_layouts_pass implementation =="
sed -n '110,270p' src/ir/transforms/resolve_backend_op_layouts_pass.cpp
echo "== backend.h relevant declarations =="
sed -n '70,145p; 235,255p' include/pypto/backend/common/backend.h
echo "== RegisterElementwiseOps rest =="
sed -n '360,560p' src/backend/common/pto_ops_elementwise.cpp
echo "== GetExprTypeAnnotation implementation candidates =="
rg -n "GetExprTypeAnnotation|GetCurrentResultTileBufTypeString|EmitInsOuts|GetExprAsCode" include src -S | head -200
echo "== read header files for pto codegen likely locations =="
fd -e h . include | rg 'codegen|pto|backend' | head -80
echo "== read likely header contents =="
fd -i 'codegen.*\.h|pto.*\.h' include src include | xargs -r -n1 sh -c 'echo "--- $0"; sed -n "1,220p" "$0"' | head -500Repository: hw-native-sys/pypto
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend.cpp finalize and lookup =="
sed -n '420,450p; 500,525p' src/backend/common/backend.cpp
echo "== registration macro and setter behavior =="
sed -n '130,235p' include/pypto/backend/common/backend.h
sed -n '460,505p' src/backend/common/backend.cpp
echo "== pto type annotation implementation =="
sed -n '1780,1895p' src/codegen/pto/pto_codegen.cpp
echo "== current result target/type implementation =="
sed -n '1860,1905p; 1360,1430p' src/codegen/pto/pto_codegen.cpp
echo "== layout predicates =="
rg -n "class BackendOpRegistryEntry|BackendOpRegistryEntry::~|BackendOpRegistryEntry::.*;" include src pypto backend -S
rg -n "RequiresRowMajor|IsRowMajor|TileLayout::row_major|SetCurrentResult" include src pypto backend -S | head -200
echo "== pto op declarations for concat_idx =="
rg -n "concat_idx|tile\.concat|mrgsort_format[12]|mrgsort_format" src include -SRepository: hw-native-sys/pypto
Length of output: 17400
Add row-max layout constraints to the custom registration.
tile.concat_idx is registered as an anonymous custom codegen slot, so it gets no BackendTileLayoutSpec: inputs 0..3 and the result tile can keep arbitrary block layout, and the row-sensitive pto.tconcatidx lowering can receive col-major operands. Chain the layout constraints on the registration entry.
🤖 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 `@src/backend/common/pto_ops_elementwise.cpp` around lines 511 - 512, Update
the tile.concat_idx registration to attach the BackendTileLayoutSpec row-max
constraints for input operands 0..3 and the result tile, while preserving the
existing MakeConcatIdxCodegenPTO callback.
| const auto is_index_dtype = [](DataType dtype) { | ||
| return dtype == DataType::INT8 || dtype == DataType::UINT8 || dtype == DataType::INT16 || | ||
| dtype == DataType::UINT16 || dtype == DataType::INT32 || dtype == DataType::UINT32; | ||
| }; | ||
| CHECK(is_data_dtype(src0->dtype_)) | ||
| << "tile.concat_idx requires 8/16/32-bit integer, FP16, BF16, or FP32 data tiles"; | ||
| CHECK(idx0->dtype_ == idx1->dtype_ && is_index_dtype(idx0->dtype_)) | ||
| << "tile.concat_idx requires matching 8/16/32-bit integer index tiles"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'DataType::UINT(8|16|32)|tile\.concat_idx|pto\.tconcatidx' \
src/ir/op/tile_ops/transform.cpp src/backend/common/pto_ops_elementwise.cppRepository: hw-native-sys/pypto
Length of output: 2079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== transform.cpp relevant section =="
sed -n '900,995p' src/ir/op/tile_ops/transform.cpp
echo "== pto_ops_elementwise.cpp relevant sections =="
sed -n '130,175p' src/backend/common/pto_ops_elementwise.cpp
sed -n '490,525p' src/backend/common/pto_ops_elementwise.cpp
echo "== dtype formatting / tconcatidx references =="
rg -n "DataType::UINT|UINT|to_dtype|dtype_name|ui8|ui16|ui32|tconcatidx|concat_idx" src backend tools include lib -g '*.{cpp,h,hpp,py,cc,cu,txt,json}' | head -n 200Repository: hw-native-sys/pypto
Length of output: 24569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== generate_expr_type_annotation implementation =="
sed -n '90,155p' src/codegen/codegen_base.cpp
echo "== tile concat_idx type annotations in codebase =="
python3 - <<'PY'
from pathlib import Path
import re
for path in Path('.').rglob('*'):
if path.is_file() and path.suffix in {'.cpp', '.hpp', '.h', '.py', '.tt':True}:
txt = path.read_text(errors='ignore')
if 'tile.concat_idx' in txt or 'DeduceTileConcatIdxType' in txt:
# Print concise context for each file
print(f"\n--- {path} ---")
lines = txt.splitlines()
for i,l in enumerate(lines,1):
if 'tile.concat_idx' in l or 'DeduceTileConcatIdxType' in l:
lo=max(1,i-8); hi=min(len(lines),i+14)
print(f"@@ {lo}-{hi}")
for j in range(lo,hi+1):
print(f"{j}: {lines[j-1]}")
break
PY
echo "== any explicit PTOAS tconcatidx dtype expectations (including docs/schema) =="
rg -n "tconcatidx|concat_idx|tile\.concat_idx|ui8|ui16|ui32|PTOAS|ptoas" . \
-g '*.{md,txt,json,yaml,yml,py,pyi,cc,cpp,h,hpp,cu,cuh,h}' \
| head -n 200Repository: hw-native-sys/pypto
Length of output: 2885
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CodegenBase::GetExprTypeAnnotation definitions/usages =="
rg -n "GetExprTypeAnnotation|EmitInsOutsClause|GenerateInsOutsClause" src include | head -n 200
echo "== pto codegen helper around tile instruction emission =="
sed -n '160,320p' src/codegen/pto/pto_scalar_expr_codegen.cpp
rg -n "i8|i16|i32|ui8|ui16|ui32|tile" src/codegen/pto/pto_scalar_expr_codegen.cpp | head -n 160
echo "== fixed file scan for tile.concat_idx =="
python3 - <<'PY'
from pathlib import Path
for path in Path('.').rglob('*'):
if path.is_file() and path.suffix not in {'.lock'}:
txt = path.read_text(errors='ignore')
if 'tile.concat_idx' in txt or 'DeduceTileConcatIdxType' in txt:
lines = txt.splitlines()
print(f"\n--- {path} ---")
for i,l in enumerate(lines,1):
if 'tile.concat_idx' in l or 'DeduceTileConcatIdxType' in l:
for j in range(max(1,i-6), min(len(lines),i+8)+1):
print(f"{j}: {lines[j-1]}")
PYRepository: hw-native-sys/pypto
Length of output: 33603
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== PTO getTypeString for integer dtypes =="
sed -n '15,80p' src/codegen/pto/pto_type_utils.cpp
echo "== GenerateInsOutsClause implementation =="
sed -n '300,380p' src/backend/common/pto_ops_shared.cpp
echo "== Test concat_idx dtype construction =="
sed -n '40,70p' tests/ut/ir/operators/test_concat_idx.py
sed -n '490,525p' tests/ut/codegen/test_pto_codegen.py
echo "== Behavioral extraction of unsigned index annotations (read-only source text) =="
python3 - <<'PY'
from pathlib import Path
text = Path("src/ir/op/tile_ops/transform.cpp").read_text()
if ("return dtype == DataType::INT8 || dtype == DataType::UINT8 || dtype == DataType::INT16 ||\n dtype == DataType::UINT16 || dtype == DataType::INT32 || dtype == DataType::UINT32" in text):
print("is_index_dtype accepts UINT8, UINT16, UINT32")
text = Path("src/codegen/pto/pto_type_utils.cpp").read_text()
for dtype in ("UINT8", "UINT16", "UINT32"):
if f"\"ui{dtype[4].lower()}{dtype[5:] if len(dtype)==3 else int(dtype[4:])}\"" in text or f'return "ui{dtype[4].lower()}{dtype[5:] if len(dtype)==3 else int(dtype[4:])}"' in text:
print(f"ptype_map emits {dtype} as ui{dtype[4].lower()}{dtype[5:] if len(dtype)==3 else int(dtype[4:])}")
text = Path("src/backend/common/pto_ops_elementwise.cpp").read_text()
print("MakeConcatIdxCodegenPTO uses GetExprTypeAnnotation:", "GetExprTypeAnnotation(op->args_[i])" in text)
PYRepository: hw-native-sys/pypto
Length of output: 8108
🌐 Web query:
PTOAS tile.buf.type ui8 i16 i32 tconcatidx unsigned tile types
💡 Result:
In the context of the PTOAS (PTO Advanced System) project, tile.buf.type and associated parameters relate to the representation of scratchpad memory tiles within the pto MLIR dialect [1][2]. The !pto.tile_buf type is a core component of this dialect, representing a local memory buffer with metadata including placement (e.g., vec, mat), element data type, dimensions, and layout configurations [1]. Regarding your specific keywords: 1. Element Types: The dtype parameter within !pto.tile_buf supports various elemental data types, including i1, i8, i16, i32, f16, f32, and bf16 [1]. These define the size and format of the individual data elements stored within the tile buffer [1]. 2. Tile Types and Indexing: - The dialect uses !pto.tile_buf for logical buffers in Destination-Passing Style (DPS) operations [2]. - While tconcatidx is not explicitly documented as a standard primitive keyword in the primary PTOAS IR manual, the dialect frequently utilizes tiling, sub-viewing (e.g., pto.tile_subview), and concatenation operations to manage tensor data [3][1][2]. In broader MLIR tensor dialects, concatenation indices are often specified to handle merging operations across dynamic or static dimensions [4]. - Physical and logical attributes, such as rows, cols, v_row (valid rows), and v_col (valid columns), are used to define the actual storage and valid region of the tile [1]. The system is designed to handle explicit pipeline synchronization and scratchpad memory management, with IR structures often requiring explicit configuration attributes like blayout (base layout) and slayout (secondary layout) [1][2].
Citations:
- 1: https://github.com/hw-native-sys/PTOAS/blob/8863dbad/docs/PTO_IR_manual.md
- 2: https://deepwiki.com/hw-native-sys/PTOAS
- 3: https://github.com/hw-native-sys/PTOAS/blob/8863dbad/include/PTO/IR/PTOOps.td
- 4: https://google.github.io/mlir-hs/mlir-hs-0.1.0.0/MLIR-AST-Dialect-Tensor.html
Reject or normalize unsigned index tiles before PTO emission.
tile.concat_idx currently accepts UINT8/16/32, but the PTO codegen emits them as ui8/ui16/ui32 via GetExprTypeAnnotation, and PTOAS tile operands take signed integer dtype spelling. Restrict the backend contract to signed INT8/16/32, or introduce PTO-safe signed casts for the index operands before lowering.
🤖 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 `@src/ir/op/tile_ops/transform.cpp` around lines 938 - 945, Update the
tile.concat_idx validation around is_index_dtype and the idx0/idx1 checks to
reject UINT8, UINT16, and UINT32 index tiles before PTO emission; accept only
matching signed INT8, INT16, or INT32 index dtypes while preserving the existing
source-data validation.
| for (const auto& type : {src0, src1, idx0, idx1}) { | ||
| CHECK(ProveValidExtentEqual(type->shape_[0], dst->shape_[0]) == ProofResult::kTrue) | ||
| << "tile.concat_idx requires all physical row counts to match dst"; | ||
| } | ||
| const auto dst_valid = GetValidShape(dst); | ||
| for (const auto& type : {src0, src1, idx0, idx1}) { | ||
| const auto valid = GetValidShape(type); | ||
| CHECK(ProveValidExtentEqual(valid[0], dst_valid[0]) == ProofResult::kTrue) | ||
| << "tile.concat_idx requires all valid row counts to match dst"; | ||
| } | ||
| const auto idx0_valid = GetValidShape(idx0); | ||
| const auto idx1_valid = GetValidShape(idx1); | ||
| auto one = std::make_shared<ConstInt>(1, DataType::INDEX, Span::unknown()); | ||
| CHECK(ProveValidExtentEqual(idx0_valid[1], one) == ProofResult::kTrue && | ||
| ProveValidExtentEqual(idx1_valid[1], one) == ProofResult::kTrue) | ||
| << "tile.concat_idx requires index tiles with valid_shape columns equal to 1"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'requires all physical row counts|valid_shape columns equal to 1|ProveValidExtentEqual' \
src/ir/op/tile_ops/transform.cppRepository: hw-native-sys/pypto
Length of output: 657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the function around the reported checks and nearby implementation.
sed -n '880,995p' src/ir/op/tile_ops/transform.cpp
# Search for tile.concat_idx implementation, shape validation docs / tests / usages.
rg -n "tile\.concat_idx|concat_idx|tile_concat_idx|TileOps|valid_shape" src tests -S | head -n 200Repository: hw-native-sys/pypto
Length of output: 28172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the concat_idx lowering/semantics and index operands to determine whether shape_[0]
# and valid_shape[1] constraints are tied to actual use.
rg -n "tile\.concat_idx|concat_idx|GetValidShape|InheritTileViewLayout|DeduceTileConcatIdxType" src -S
sed -n '960,1080p' src/backend/common/pto_ops_elementwise.cpp
rg -n "src0_idx|src1_idx|x0|x1|concat_idx|cat" src backend codegen -S | head -n 240Repository: hw-native-sys/pypto
Length of output: 35782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '130,185p' src/backend/common/pto_ops_elementwise.cpp
rg -n "MakeConcatIdxCodegenPTO|padd|concat|idx|src0_idx|src1_idx|x0|x1|col" src/backend/src -S 2>/dev/null || true
rg -n "concat_idx|tconcatidx|tile.create|tile.concat_idx" tests src -S | head -n 200Repository: hw-native-sys/pypto
Length of output: 27364
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect tile.concat_idx behavior tests and the relevant invalid-index contract test shape.
sed -n '1,120p' tests/ut/ir/operators/test_concat_idx.py
sed -n '300,355p' tests/ut/ir/operators/test_concat_idx.py
sed -n '10,360p' tests/st/runtime/ops/test_concat_idx.py
# Inspect tile type helper definitions.
sed -n '1,130p' src/ir/type.cpp
rg -n "GetValidShape|tile_view_semantics::GetEffectiveTileView|ProveValidExtentEqual|InheritTileViewLayout" src/ir src/backend common -S | head -n 180Repository: hw-native-sys/pypto
Length of output: 31925
Validate canonical valid extents instead of storage shape counts.
src0/src1/dst only need matching valid row counts; rejecting allocated rows larger than dst blocks valid valid-row inputs. Likewise, idx0/idx1 only need at least one valid index column, but the equality check rejects valid_shape[1] >= 2. Remove the physical-row checks and change the index-column check to >= 1.
🤖 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 `@src/ir/op/tile_ops/transform.cpp` around lines 949 - 964, In the
tile.concat_idx validation block, remove the physical row-count loop that
compares type->shape_[0] with dst->shape_[0]. Retain validation using
GetValidShape for matching valid row counts, and update the
idx0_valid/idx1_valid column validation to require each valid column extent to
be at least one rather than exactly equal to one.
| REGISTER_OP("tile.concat_idx") | ||
| .set_op_category("TileOp") | ||
| .set_description("Indexed per-row concatenation into a destination tile") | ||
| .add_argument("src0", "First source tile") | ||
| .add_argument("src1", "Second source tile") | ||
| .add_argument("src0_idx", "Per-row source-0 column count") | ||
| .add_argument("src1_idx", "Per-row source-1 column count") | ||
| .add_argument("dst", "Destination tile") | ||
| .set_input_memory(0, MemorySpace::Vec) | ||
| .set_input_memory(1, MemorySpace::Vec) | ||
| .set_input_memory(2, MemorySpace::Vec) | ||
| .set_input_memory(3, MemorySpace::Vec) | ||
| .set_input_memory(4, MemorySpace::Vec) | ||
| .set_output_memory(MemorySpace::Vec) | ||
| .set_output_reuses_input(4) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'tile\.concat_idx|set_input_layout|set_output_layout' \
src/backend/common/pto_ops_elementwise.cppRepository: hw-native-sys/pypto
Length of output: 1288
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file around REGISTER_OP tile.concat_idx =="
sed -n '950,1000p' src/ir/op/tile_ops/transform.cpp
echo
echo "== candidate helpers near REGISTER_OP =="
rg -n 'void REGISTER_OP|struct .*Register|class .*Register|set_input_memory\(|set_input_layout\(|set_output_layout\(|add_argument' src/ir/op/tile_ops/transform.cpp src/ir/op -g'*.cpp' -g'*.h' | head -220Repository: hw-native-sys/pypto
Length of output: 25491
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== define/register helper declarations/usages for REGISTER_OP =="
rg -n 'REGISTER_OP\b|struct .*OpRegistry|class .*OpRegistry|OpRegistry|RegEntry' src/ir/op src/include -g'*.cpp' -g'*.h' | head -200
echo
echo "== locate simple ops code =="
rg -n 'kSimpleOps|set_input_layout\\(|set_output_layout\\(' src -g'*.cpp' -g'*.h' | head -200Repository: hw-native-sys/pypto
Length of output: 14226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tile concat idx in backend =="
rg -n 'concat_idx|tconcatidx|kSimpleOps|SrcSimpleOps|SimpleOps|tile\.concat_idx|set_input_layout\(' src/backend src/include -g'*.cpp' -g'*.h' -g'*.py' 2>/dev/null | head -250
echo
echo "== op registry/map definition candidates =="
rg -n 'typedef|class|struct.*OpRegistry|OpRegistry::|GetOp\\(|SimpleOp|SrcSimpleOps|kSimpleOps' src/backend src -g'*.cpp' -g'*.h' | head -250
echo
echo "== precise backend occurrences without include path dependency =="
find src -type f \
\( -name '*.cpp' -o -name '*.h' \) -print0 \
| xargs -0 rg -n 'tile\.concat_idx|tconcatidx|set_input_layout\\(|set_output_layout\\(|kSimpleOps|SrcSimpleOps|SimpleOp' | head -300Repository: hw-native-sys/pypto
Length of output: 2835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pto_ops_elementwise around codegen and kSimpleOps =="
sed -n '1,180p' src/backend/common/pto_ops_elementwise.cpp
echo
sed -n '360,535p' src/backend/common/pto_ops_elementwise.cpp
echo
echo "== BackendOpRegistryEntry layout API =="
sed -n '460,500p' src/backend/common/backend.cppRepository: hw-native-sys/pypto
Length of output: 18083
Register row-major constraints for tile.concat_idx.
tile.concat_idx is registered with only the PTO codegen handler, so operands that should go to pto.tconcatidx are not required to be row-major and can fail assembler verification. Add row-major set_input_layout for inputs 0-4 and set_output_layout for the result.
🤖 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 `@src/ir/op/tile_ops/transform.cpp` around lines 971 - 985, Update the
tile.concat_idx registration to add row-major set_input_layout constraints for
inputs 0 through 4 and a row-major set_output_layout constraint for the result.
Keep the existing memory-space and output-reuse settings unchanged.
| def test_pto_codegen_concat_idx_uses_four_inputs_and_reused_destination(): | ||
| """Indexed concat emits the exact PTO op with ``dst`` only as its output.""" | ||
|
|
||
| @pl.program | ||
| class ConcatIdxProgram: | ||
| @pl.function(type=pl.FunctionType.InCore) | ||
| def kernel( | ||
| self, | ||
| src0: pl.Tensor[[8, 64], pl.FP32], | ||
| src1: pl.Tensor[[8, 64], pl.FP32], | ||
| idx0: pl.Tensor[[8, 8], pl.INT32], | ||
| idx1: pl.Tensor[[8, 8], pl.INT32], | ||
| out: pl.Tensor[[8, 64], pl.FP32], | ||
| ) -> pl.Tensor[[8, 64], pl.FP32]: | ||
| value0 = pl.load(src0, [0, 0], [8, 64]) | ||
| value1 = pl.load(src1, [0, 0], [8, 64]) | ||
| count0 = pl.load(idx0, [0, 0], [8, 8], valid_shapes=[8, 1]) | ||
| count1 = pl.load(idx1, [0, 0], [8, 8], valid_shapes=[8, 1]) | ||
| dst = pl.load(out, [0, 0], [8, 64]) | ||
| result = pl.tile.concat_idx(value0, value1, count0, count1, dst) | ||
| return pl.store(result, [0, 0], out) | ||
|
|
||
| lines = _get_mlir_lines(_generate_default_mlir(ConcatIdxProgram)) | ||
| concat_line = _single_line(lines, "pto.tconcatidx") | ||
| assert "ins(" in concat_line | ||
| assert "outs(" in concat_line |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm exact ins/outs ordering and destination-reuse emission for concat_idx
rg -n -B3 -A40 'concat_idx' src/backend/common/pto_ops_elementwise.cppRepository: hw-native-sys/pypto
Length of output: 2767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the concat_idx codegen implementation and the nearby test behavior.
fd -a 'pto_ops_elementwise\.cpp|test_pto_codegen\.py' . | sed 's#^\./##'
echo '--- outline candidate files ---'
ast-grep outline src/backend/common/pto_ops_elementwise.cpp --match MakeConcatIdxCodegenPTO --view expanded || true
echo '--- MakeConcatIdxCodegenPTO body ---'
rg -n -B10 -A80 'MakeConcatIdxCodegenPTO' src/backend/common/pto_ops_elementwise.cpp
echo '--- test section ---'
sed -n '470,540p' tests/ut/codegen/test_pto_codegen.pyRepository: hw-native-sys/pypto
Length of output: 14388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether tile.concat_idx is explicitly declared as a 4-input op so the test's
# "four inputs" framing is grounded in the backend IR contract.
rg -n -B5 -A12 'set_op_declaration|TileConcatIdx|concat_idx|tconcatidx' src testsRepository: hw-native-sys/pypto
Length of output: 23158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Deterministically probe how the regex assertion would treat representative emitted lines,
# including malformed outputs, many inputs, outputs, and a reused destination that is absent from alloc text.
python3 - <<'PY'
import re
cases = {
"expected_format": 'pte_call "%pt" %tile_1 = pto.tconcatidx %tile_0a, %tile_0b, %tile_1a, %tile_1b : ins(%tile_0a, %tile_0b, %tile_1a, %tile_1b) outs(%tile_1) -> tensor<...>',
"no_ins": 'pte_call "%pt" %tile_1 = pto.tconcatidx %tile_0a, %tile_0b, %tile_1a, %tile_1b : outs(%tile_1) -> tensor<...>',
"no_outs": 'pte_call "%pt" %tile_1 = pto.tconcatidx %tile_0a, %tile_0b, %tile_1a, %tile_1b : ins(%tile_0a, %tile_0b, %tile_1a, %tile_1b) -> tensor<...>',
"extra_input": 'pte_call "%pt" %tile_1 = pto.tconcatidx %tile_tmp, %tile_0a, %tile_0b, %tile_1a, %tile_1b : ins(%tile_tmp, %tile_0a, %tile_0b, %tile_1a, %tile_1b) outs(%tile_1) -> tensor<...>',
"extra_output": 'pte_call "%pt" %tile_x = pto.tconcatidx %tile_0a, %tile_0b, %tile_1a, %tile_1b : ins(%tile_0a, %tile_0b, %tile_1a, %tile_1b) outs(%tile_1, %tile_tmp) -> tensor<...>',
"reused_dst_not_from_alloc": 'pte_call "%pt" %unrelated_tile = pto.tconcatidx %tile_0a, %tile_0b, %tile_1a, %tile_1b : ins(%tile_0a, %tile_0b, %tile_1a, %tile_1b) outs(%unrelated_tile) -> tensor<...>',
}
line, alloc_tail = cases["reused_dst_not_from_alloc"], "tile.buf[4] dst_tile = alloc <8, 64> ..."
for name, line in cases.items():
has_ins = "ins(" in line
has_outs = "outs(" in line
ins_match = re.search(r"ins\(([^)]*)\)", line)
outs_match = re.search(r"outs\(([^)]*)\)", line)
ins_operands = [o.strip() for o in ins_match.group(1).split(",")] if ins_match else []
outs_operands = [o.strip() for o in outs_match.group(1).split(",")] if outs_match else []
print(f"{name}: ins={has_ins} outs={has_outs} ins_count={len(ins_operands)} ins={ins_operands} outs_count={len(outs_operands)} outs={outs_operands}")
if outs_operands:
print(f" outs references dst tile_buf allocation: {'tile.buf' in alloc_tail and outs_operands[0].split(':')[0].strip() in alloc_tail}")
PYRepository: hw-native-sys/pypto
Length of output: 1155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Ins/Outs clause helpers and any existing patterns for verifying operand identities in this test suite.
rg -n -B5 -A25 'EmitInsOuts|GenerateInsOutsClause|GetCurrentResultBuf|set_output_reuses_input|_single_line|_get_alloc_tile_lines' src backend tests/ut/codegen
# Print nearby EmitInsOuts helper definition with line numbers.
rg -n 'EmitInsOuts|GenerateInsOutsClause' src/backend/common/pto_ops_elementwise.cpp
sed -n '1,150p' src/backend/common/pto_ops_elementwise.cppRepository: hw-native-sys/pypto
Length of output: 50376
Exercise the pto.tconcatidx in/out contract against the emitted MLIR.
MakeConcatIdxCodegenPTO emits args_[0..3] in ins() while tile.concat_idx has args_[4] as the reused destination. Check that the emitted line contains exactly those four inputs, a single outs() operand, and that the outs() operand is the same SSA used by the dst load/result target rather than just the presence of ins(/outs(.
🤖 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 `@tests/ut/codegen/test_pto_codegen.py` around lines 498 - 523, Strengthen
test_pto_codegen_concat_idx_uses_four_inputs_and_reused_destination to validate
the emitted pto.tconcatidx contract: assert ins() contains exactly the four
source/index operands, outs() contains exactly one operand, and that operand
matches the SSA value loaded into dst and passed as the reused
destination/result target. Replace the current presence-only assertions while
preserving the existing program setup.
| @pytest.mark.parametrize( | ||
| "index_dtype", | ||
| [ | ||
| DataType.INT8, | ||
| DataType.UINT8, | ||
| DataType.INT16, | ||
| DataType.UINT16, | ||
| DataType.INT32, | ||
| DataType.UINT32, | ||
| ], | ||
| ) | ||
| def test_concat_idx_returns_destination_type(data_dtype, index_dtype): | ||
| src0 = _tile("src0", (8, 64), data_dtype, (8, 64)) | ||
| src1 = _tile("src1", (8, 64), data_dtype, (8, 48)) | ||
| idx0 = _tile("idx0", (8, 8), index_dtype, (8, 1)) | ||
| idx1 = _tile("idx1", (8, 8), index_dtype, (8, 1)) | ||
| dst = _tile("dst", (8, 64), data_dtype, (8, 64)) | ||
|
|
||
| call = tile.concat_idx(src0, src1, idx0, idx1, dst) | ||
|
|
||
| assert isinstance(call.type, ir.TileType) | ||
| assert isinstance(dst.type, ir.TileType) | ||
| assert call.type.dtype == data_dtype | ||
| assert call.type.shape == dst.type.shape |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Test locks in acceptance of unsigned index dtypes that PTOAS rejects.
This parametrizes index_dtype over UINT8/UINT16/UINT32 and asserts concat_idx succeeds, but the canonical PTOAS verifier rejects ui* types for the index operands — they must be safely lowered to signless i8/i16/i32 or rejected outright. As written, this test certifies behavior that will fail downstream at the PTOAS verifier.
Once the underlying contract (in the IR op definition) is fixed to reject or coerce unsigned index dtypes, this parametrize list needs to change accordingly (either drop the unsigned entries or assert a ValueError/coercion instead of success).
🤖 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 `@tests/ut/ir/operators/test_concat_idx.py` around lines 41 - 64, Update
test_concat_idx_returns_destination_type to match the PTOAS index-type contract:
remove UINT8, UINT16, and UINT32 from the successful index_dtype
parameterization, or change those cases to assert the IR op’s documented
rejection/coercion behavior. Keep signed index dtype coverage and destination
type assertions unchanged.
| def test_concat_idx_rejects_invalid_index_contract(): | ||
| src = _tile("src", (8, 64), DataType.FP32, (8, 64)) | ||
| dst = _tile("dst", (8, 64), DataType.FP32, (8, 64)) | ||
| bad_dtype = _tile("bad", (8, 8), DataType.FP32, (8, 1)) | ||
| bad_cols = _tile("wide", (8, 8), DataType.INT32, (8, 2)) | ||
| good = _tile("good", (8, 8), DataType.INT32, (8, 1)) | ||
|
|
||
| with pytest.raises(ValueError, match="integer index"): | ||
| tile.concat_idx(src, src, bad_dtype, good, dst) | ||
| with pytest.raises(ValueError, match="columns equal to 1"): | ||
| tile.concat_idx(src, src, bad_cols, good, dst) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforces an overly strict idx.valid_cols == 1 contract; canonical requirement is >= 1.
bad_cols uses a valid_shape of (8, 2) and this test asserts it must raise "columns equal to 1". Per the canonical shape contract, only idx.valid_cols >= 1 (plus matching valid rows) is required — valid_cols == 2 should be legal. This test pins the current (too-strict) validation as correct, which will need to be inverted (asserting success, not failure) once the shape check is relaxed.
🤖 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 `@tests/ut/ir/operators/test_concat_idx.py` around lines 67 - 77, Update
test_concat_idx_rejects_invalid_index_contract so an index tensor with
valid_cols == 2, represented by bad_cols, is accepted rather than expected to
raise. Remove the columns-equal-to-1 failure assertion and verify concat_idx
succeeds with the existing matching-row inputs, while retaining the invalid
integer-index assertion.
2 participants
Summary
tile.concat_idxIR, Python IR/DSL APIs, destination-reuse type contract, and exactpto.tconcatidxloweringtile.fillpad_inplacelowering from legacypto.tfillpadto exactpto.tfillpad_inplaceValidation
cmake --build build --parallel 1pto.tfillpad_inplacetask-submit validation: zero/max/min modes, 3/3 passedpto.tconcatidx: PTOAS v0.53 compilation succeeds; two isolated A2/A3 executions (boundary-free legal counts included) reproducibly raiseCCU instruction address check errorand schedulerS1:running-stalled, so same-name hardware ST remains blocked and is documented as suchNotes
This branch is based directly on upstream
mainand is independent of the earlier PTOAS batch PRs.