Skip to content

fix: canonicalize singleton partition-view axes for GT lowering#463

Open
TaoTao-real wants to merge 3 commits intohw-native-sys:mainfrom
TaoTao-real:codex/issue453-singleton-subview-reorder
Open

fix: canonicalize singleton partition-view axes for GT lowering#463
TaoTao-real wants to merge 3 commits intohw-native-sys:mainfrom
TaoTao-real:codex/issue453-singleton-subview-reorder

Conversation

@TaoTao-real
Copy link
Copy Markdown
Contributor

Summary

  • Add a safe canonicalization in memref.subview -> GlobalTensor lowering for partition-view outputs.
  • Reorder only statically-proven singleton axes (size == 1) ahead of non-singleton axes when building GT shape/stride metadata.
  • Keep dynamic axes as non-singleton (no speculative reorder).
  • Add regression test covering both positive and negative paths.

Motivation

Design

  • Location: SubviewToEmitCPattern in lib/PTO/Transforms/PTOToEmitC.cpp.
  • Safety boundary:
    • Reorder is enabled only when the number of non-singleton axes is <= 2.
    • Only axes statically known as singleton are moved.
    • Non-singleton axis relative order is preserved.
  • This is metadata canonicalization only (shape/stride ordering); pointer/offset address computation is unchanged.

Testing

  • Added: test/basic/issue453_partition_view_singleton_axis_reorder.pto
    • Positive: [1,16,1,16] lowers to Shape<1,1,1,16,16>.
    • Negative: [2,16,1,16] remains non-reordered (Shape<1,2,16,1,16>).
  • Ran:
    • build/tools/ptoas/ptoas --pto-arch=a5 test/basic/issue453_partition_view_singleton_axis_reorder.pto | FileCheck ...
    • build/tools/ptoas/ptoas test/basic/infer_layout_ambiguous_minor2d_user_dn.pto | FileCheck ...
    • build/tools/ptoas/ptoas --pto-arch=a3 test/basic/tstore_forms_emitc.pto | FileCheck ...

Risk / Rollback

  • Risk: low, scoped to subview->GlobalTensor metadata construction.
  • Rollback: revert this PR to restore previous lowering behavior.

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 introduces a singleton axis reordering optimization for partition views in PTOToEmitC.cpp, allowing axes with a size of 1 to be moved to the front when safe. It also includes a new test case to verify this behavior. I have no further feedback to provide, as the suggested improvements for using standard library algorithms are optional and the current implementation is clear and correct.

@TaoTao-real
Copy link
Copy Markdown
Contributor Author

Update with the refined scope discussed:

  • singleton-axis canonicalization for partition-view subview is now selectively enabled only when:
    1. consumer is pto.tload
    2. source GlobalTensor layout is ND or DN
    3. destination tile config is NZ-like
  • it no longer runs globally for all subviews.

Guard coverage added in test/basic/issue453_partition_view_singleton_axis_reorder.pto:

Local validation:

  • ninja -C build ptoas
  • build/tools/ptoas/ptoas --pto-arch=a5 test/basic/issue453_partition_view_singleton_axis_reorder.pto | FileCheck --check-prefix=A5
  • build/tools/ptoas/ptoas test/basic/infer_layout_ambiguous_minor2d_user_dn.pto | FileCheck
  • build/tools/ptoas/ptoas --pto-arch=a3 test/basic/tstore_forms_emitc.pto | FileCheck --check-prefix=A3

@TaoTao-real
Copy link
Copy Markdown
Contributor Author

Follow-up update based on review direction (clean layering):

  • Moved singleton-axis canonicalization logic out of PTOToEmitC.cpp into a dedicated pass:
    • pto-canonicalize-subview-for-tload
    • implementation file: lib/PTO/Transforms/PTOCanonicalizeSubviewForTLoad.cpp
  • Pipeline placement:
    • after PTOViewToMemref
    • before EmitC lowering
  • PTOToEmitC now only consumes a marker attr (pto.singleton_axis_permutation) and applies the precomputed permutation; strategy/gating logic is no longer embedded in codegen lowering.

Gating semantics remain the same:

  • trigger only when all effective consumers are pto.tload
  • source GlobalTensor layout is ND/DN
  • destination tile config is NZ-like
  • and only safe singleton-axis movement (<=2 non-singleton dims)

Additional test coverage added:

  • DN -> NZ-like tload positive case in test/basic/issue453_partition_view_singleton_axis_reorder.pto

Validation rerun (all pass):

  • ninja -C build ptoas
  • ptoas --pto-arch=a5 test/basic/issue453_partition_view_singleton_axis_reorder.pto | FileCheck --check-prefix=A5
  • ptoas test/basic/infer_layout_ambiguous_minor2d_user_dn.pto | FileCheck
  • ptoas --pto-arch=a3 test/basic/tstore_forms_emitc.pto | FileCheck --check-prefix=A3

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.

[Bug] partion_tensor_view在高维tensor场景下会触发pto-isa assert

1 participant