Skip to content

Fix #249: subset valid-shape inference with dynamic offsets#257

Open
TaoTao-real wants to merge 1 commit intozhangstevenunity:mainfrom
TaoTao-real:codex/fix-issue-249-subset-validshape
Open

Fix #249: subset valid-shape inference with dynamic offsets#257
TaoTao-real wants to merge 1 commit intozhangstevenunity:mainfrom
TaoTao-real:codex/fix-issue-249-subset-validshape

Conversation

@TaoTao-real
Copy link
Contributor

Summary

  • Fix pto.subset return type inference so valid-shape inference does not depend on subset offsets.
  • Add regression test test/basic/subset_infer_dynamic_offset.mlir to cover dynamic offset + static result type.

Motivation

Design

  • In SubsetOp::inferReturnTypes, derive result validShape only from:
    • static sizes
    • parent tile validShape
  • Explicitly remove offset-dependent valid-shape derivation.
  • Invariant: dynamic offsets must not force static valid dims to become dynamic.

Testing

  • Local targeted check:
    • /tmp/PTOAS_upstream_compare_20260312_165206/build/tools/ptoas/ptoas test/basic/subset_infer_dynamic_offset.mlir (pass)
  • Remote full-board validation on 101.245.68.6 (2026-03-12):
    • Upstream main baseline: OK=150 FAIL=3 SKIP=7
    • main + this fix: OK=150 FAIL=3 SKIP=7
    • FAIL set unchanged (plan_memory_scopes_independent, prelu, print), no new regressions.

Risk / Rollback

  • Risk: low, change is localized to subset return type inference.
  • Rollback: revert this commit.

Review Focus

  • Verify SubsetOp::inferReturnTypes no longer uses offsets to derive valid dims.
  • Verify dynamic-offset subset with static result type now passes.
  • Verify no regressions in full-board run relative to upstream baseline.

@zhangstevenunity
Copy link
Owner

@codex

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: e0733a11f2

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

} else {
vdim = kDynamicValidDim;
}
vdim = std::min<int64_t>(sizeDim, pv);

Choose a reason for hiding this comment

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

P1 Badge Preserve static-offset clipping in subset valid-shape inference

The new inference path ignores subset offsets entirely and always uses min(size, parentValid), which regresses constant-offset cases where the valid region should shrink. For example, with parent v_col=32, sizes=[...,32], and a static col offset of 16, this now infers v_col=32 instead of 16; that overstates validity and diverges from the lowering path (computeSubsetValidDim in PTOViewToMemref) that still applies offset-based clipping. This can make earlier IR passes treat padded elements as valid and produce inconsistent metadata across the pipeline.

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.

pto.subset dynamic offset causes inferred v_col=? vs declared static valid mismatch

2 participants