fix(vmi): propagate layouts through integer negation - #1407
Closed
chenshengxin2026 wants to merge 1 commit into
Closed
chenshengxin2026 wants to merge 1 commit into
chenshengxin2026 wants to merge 1 commit into
Conversation
chenshengxin2026
force-pushed
the
fix/issue-1373-vmi-negi-layout
branch
from
September 1, 2026 04:22
972e94c to
ec49c53
Compare
4 tasks
chenshengxin2026
force-pushed
the
fix/issue-1373-vmi-negi-layout
branch
from
September 4, 2026 01:25
1b2cbff to
199c6b8
Compare
The unified integer negation lowers to pto.vmi.negi before layout assignment, but VMINegIOp was missing from isSameLayoutOp in VMILayoutPropagation.cpp. Layout propagation therefore had no transfer for negi and silently placed no constraint between its source and result, letting the two ports be assigned incompatible layouts. That tripped the VMI verifier reported in issue hw-native-sys#1373: 'pto.vmi.negi' op requires all layout-assigned VMI data values to have the same layout Treat VMINegIOp as a same-layout VMI op during layout propagation. This restores the op's own invariant -- VMINegIOp::verify() already calls verifyAllSameVRegShapeAndLayout with requireSameElement -- so the constraint is exactly as strong as the op requires, not stronger. It also makes the four layout passes agree again: VMINegIOp was already registered in VMILayoutAssignment.cpp, VMILayoutRematerialize.cpp and VMILayoutSinkMaterialization.cpp, and propagation was the only one missing it. Add regression coverage for the six integer non-mask cases from the issue (i8 vl1/vl64/vl128, i16 vl1/vl64, i32 vl1) with two RUN lines: - A pto-test-opt run that stops right after layout assignment. This is the primary regression, matching the dominant convention in test/lit/vmi_new, and it reproduces all six failures directly rather than aborting on the first one. It asserts the concrete, non- contiguous layout on both negi ports, so the coverage cannot silently lapse if a future change collapses these cases to plain contiguous. - A full ptoas --emit-vpto run proving the pipeline completes, guarded by --implicit-check-not so the residual-VMI check covers the whole output instead of only the region after the last match. This run needs provably 32-byte aligned dynamic offsets, otherwise physical read legalization fails first and masks the layout regression. Verified against a build carrying the fix: both RUN lines fail on the pre-fix compiler and pass after it, and test/lit/vmi_new is 553/553.
chenshengxin2026
force-pushed
the
fix/issue-1373-vmi-negi-layout
branch
from
September 4, 2026 01:33
199c6b8 to
fa4e994
Compare
github-actions
Bot
force-pushed
the
main
branch
3 times, most recently
from
September 4, 2026 10:01
4687426 to
7e2ec3e
Compare
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
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
VMINegIOpas a same-layout VMI op during layout propagationvnegcases from [Bug] PTODSL-vmi,vneg 整数类型用例触发 VMI layout 不一致错误 #1373, at both the layout-assignment and full-pipeline levelsRoot cause
The unified integer
vneglowers topto.vmi.negibefore layout assignment (ptoas_pipeline.cpprunsvmi-lower-unified-to-legacyahead ofvmi-layout-assignment), butVMINegIOpwas missing fromisSameLayoutOpinVMILayoutPropagation.cpp.VMILayoutPropagator::propagateThroughis fail-open: when no transfer is registered for an op it returnssuccess()without recording any constraint. Propagation therefore had no edge betweennegi's source and result, the two ports were assigned incompatible layouts independently, and the VMI verifier rejected the result:This also explains why the identical
vnotcode reported in the issue worked:VMINotOpwas already in the list.The constraint added here is exactly as strong as the op's own contract —
VMINegIOp::verify()already callsverifyAllSameVRegShapeAndLayout(..., requireSameElement=true)— so it cannot over-constrain. It also restores consistency across the layout passes:VMINegIOpwas already registered inVMILayoutAssignment.cpp,VMILayoutRematerialize.cppandVMILayoutSinkMaterialization.cpp; propagation was the only one missing it.Tests
test/lit/vmi_new/vmi_ptoas_cli_integer_vneg_layout.ptocovers all six failing cases from the issue (i8 vl1/vl64/vl128, i16 vl1/vl64, i32 vl1) with two RUN lines:pto-test-optstopping after layout assignment — the primary regression. It isolates the layout invariant from every later stage and asserts the concrete, non-contiguous layout on bothnegiports (num_groups = 1, slots = 8for the vl1 cases,contiguous, lane_stride = 2|4for the rest), plus the full load/negi/store chain forvneg_i8_vl64. Pinning the concrete layout matters: a bare "a vneg survived" check would keep passing if a future change collapsed these cases to plain contiguous, silently retiring the coverage.ptoas --emit-vpto— proves the whole pipeline completes, guarded by--implicit-check-notso the residual-VMI check applies to the entire output rather than only the region after the last match. This run needs provably 32-byte-aligned dynamic offsets, otherwise physical read legalization fails first and masks the layout regression; the first run has no such prerequisite.Verification:
test/lit/vmi_new: 553/553 passtest/lit: no related failures (the only red test locally isnpu_validation/deepseek_score_inputs.test, which needsnumpy, absent from the local environment)PTOAS_ENABLE_WERROR=ONNote on the current CI status
vmi-source-patch-checkandbuild-and-testare red for reasons unrelated to this change; both reproduce on other open PRs and on plainmain:vmi-source-patch-check:packaging/ptoas-vmi/pyproject.toml.patchno longer applies tomain'spyproject.toml. Verified by applying the patch tomaincontent directly, with no PR involved.build-and-test: the "Validate strict CMake 4 configure" step errors on the deprecatedFetchContent_Populatecall atcmake/fetch_cann_cmake.cmake:67under CMake 4.1.3. It fails at configure time, before anything is compiled or any test is run.As a consequence CI has not yet exercised the new lit test; the results above are from local runs against a build carrying this fix.
Follow-up
#1458 tracks the underlying fragility this bug came from: the same-layout op set is hand-maintained in four separate lists, and layout propagation is fail-open for unregistered ops.
Fixes #1373