fix(emitc): avoid invalid static-to-dynamic GlobalTensor cast for MGATHER/MSCATTER - #1180
Open
FangRui0 wants to merge 3 commits into
Open
fix(emitc): avoid invalid static-to-dynamic GlobalTensor cast for MGATHER/MSCATTER#1180FangRui0 wants to merge 3 commits into
FangRui0 wants to merge 3 commits into
Conversation
The PTOToEmitCTypeConverter maps tensor_view / partition_tensor_view to a GlobalTensor opaque type with fully-dynamic Stride<-1,...> template params, because strides are not carried on the MLIR type. The static partition_view pattern, however, materializes a GlobalTensor with concrete static strides. Dialect conversion bridges the two with an unrealized_conversion_cast that the cleanup lowered to an emitc.cast, i.e. an invalid C-style cast between two GlobalTensor instantiations that have no converting constructor. The generated C++ then failed to compile for MGATHER / MSCATTER (issue hw-native-sys#1165, a v0.55 regression exposed once these ops became tensor-view-native). Recognize such structurally-compatible GlobalTensor-to-GlobalTensor bridges in the cast cleanup (identical except static-vs-dynamic Shape/Stride template params) and forward the value instead of emitting a cast, so the static-stride GlobalTensor flows directly into the templated MGATHER / MSCATTER call. Add a NOCAST FileCheck pass to the mgather/mscatter lit test guarding against re-emission of the C-style GlobalTensor cast.
Forwarding the static-stride GlobalTensor into a func/emitc return breaks verification because the enclosing function's result type is fixed to the dynamic-stride form. Only forward when every consumer accepts a more-specific template instantiation (e.g. MGATHER/MSCATTER); otherwise fall through to the emitc.cast branch. Fixes a regression in issue31_partition_view_parser_compat.
…THER/MSCATTER Per PR review: prefer fixing at the source over the sink. MGATHER/MSCATTER are template intrinsics that accept the concrete descriptor directly, so peel the type-converter materialization bridge on their mem/idx/dst (src) operands. The static-stride GlobalTensor bridge then becomes dead and is dropped by the first cast-cleanup rule (use_empty), so it never reaches the emitc.cast fallback and no invalid C-style GlobalTensor<...> cast is emitted. This removes the sink-side areRefinableGlobalTensorTypes helper and its forward/feedsReturn branch, which only tolerated the inconsistency at the exit. Bringing MGATHER/MSCATTER in line with the peel convention already used by the other operand-consuming patterns keeps the consistency contract in one place.
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
Fixes #1165 — a v0.55 regression where PTO-to-EmitC lowering emits C++ that fails to compile for
MGATHER/MSCATTER.Root cause.
TensorViewType/PartitionTensorViewTypecarry only shape + element type in the MLIR type — never strides. SoPTOToEmitCTypeConvertercan only map them to aGlobalTensoropaque type with a fully-dynamicStride<-1,-1,-1,-1,-1>template. The staticpartition_viewpattern, which does have access to the defining op, instead materializes aGlobalTensorwith concrete static strides (e.g.Stride<2048,2048,2048,32,1>). Dialect conversion bridges the two mismatchedGlobalTensortypes with anunrealized_conversion_cast, and the cleanup lowered it to anemitc.cast— an invalid C-style cast between twoGlobalTensorinstantiations that have no converting constructor:This surfaced in v0.55 once
MGATHER/MSCATTERbecame tensor-view-native and the partition-viewGlobalTensorstarted flowing directly into them (commits 61ed749, 83901f6, 8d1d8b5).Fix
In the unrealized-cast cleanup, recognize a
GlobalTensor→GlobalTensorbridge that is identical except for static-vs-dynamicShape/Stridetemplate params (areRefinableGlobalTensorTypes) and forward the value instead of emitting a cast. The static-strideGlobalTensorthen flows straight into the templatedMGATHER/MSCATTERcall, which accepts any instantiation. A C-style cast between differingGlobalTensortemplate instantiations is never valid C++, so this cleanup path should never produce one.Emitted C++ after the fix (no intermediate cast, static strides preserved):
Validation (remote A3, LLVM21 toolchain)
llvm-littest/lit/pto/mgather_mscatter_base_a3_emitc.pto— PASS, including the newNOCASTguard.ccec --cce-aicore-arch=dav-c220-vec -D__NPU_ARCH__=2201 -fsyntax-only— 0 errors; the originalno matching conversion for C-style castis gone andMGATHERinstantiates with the staticGlobalTensortype.Test plan
mgather_mscatter_base_a3_emitc.ptolit passes (adds a second FileCheck passNOCAST-NOT: (GlobalTensor<guarding against re-emission of the C-style cast)check-ptoregression on CI