Feature/gm vectorized ldg stg#897
Open
kuri780 wants to merge 20 commits into
Open
Conversation
…store) Extend verifyLdgStgAccess to accept vector<2xf16>, vector<2xbf16>, and vector<2xf32> in addition to existing scalar types. Extend buildL1CacheLoadCallee / buildL1CacheStoreCallee, getLdgCallResultType, convertLdgCallResult, and convertStgValue in both VPTOLLVMEmitter and VPTOCANN900LLVMEmitter to map vector types to hardware intrinsics by total bit width: - vector<2xf16/2xbf16> → 32-bit → s32/b32 intrinsics → i32 bitcast - vector<2xf32> → 64-bit → s64/b64 intrinsics → i64 bitcast Co-Authored-By: Claude <noreply@anthropic.com>
Positive test (simt_lowlevel_vector_ldg_stg_vpto_llvm.pto): - vector<2xf16> -> s32/b32 intrinsics + i32<->vector bitcast - vector<2xbf16> -> s32/b32 intrinsics + i32<->vector bitcast - vector<2xf32> -> s64/b64 intrinsics + i64<->vector bitcast - static constant offset and dynamic runtime offset - default cache policy Negative test (simt_lowlevel_ldst_policy_negative.pto): - Remove vector<2xf32> (now valid), keep UB addr space rejection - Add vector<4xf32>, vector<3xf16>, vector<2xi32> rejection - Add stg UB addr space rejection - Split into independent functions for isolated verification Co-Authored-By: Claude <noreply@anthropic.com>
1. Remove --implicit-check-not=error from negative tests (was conflicting
with verifier error diagnostics).
2. Split the monolithic negative test into three focused files:
- simt_lowlevel_ldst_policy_negative.pto: ldg on non-GM pointer
- simt_lowlevel_vector_ldg_stg_negative.pto: illegal vector types
(vector<4xf32>, vector<3xf16>, vector<2xi32> ldg and stg)
- simt_lowlevel_stg_policy_negative.pto: stg on non-GM pointer
3. Change positive test dynamic offset from i32 to i64:
- arith.index_castui i32->index → arith.index_cast i64->index
- Remove llvm.zext i32->i64 CHECK (no longer emitted)
- Verify runtime i64 flows directly into vector GEP
Co-Authored-By: Claude <noreply@anthropic.com>
…case 1. vector_ldg_stg_negative.pto: reduce from 4 to 1 representative case (vector<4xf32> ldg) to avoid fragile multi-error diagnostics. 2. vector_ldg_stg_vpto_llvm.pto: extract dynamic i64 offset into a dedicated function (@vector_ldg_stg_dyn_offset) with no static non-zero constants. This guarantees the llvm.getelementptr CHECK matches the runtime i64 index, not a static GEP from the main kernel. Co-Authored-By: Claude <noreply@anthropic.com>
- Add f32x2 = VectorType.get([2], F32Type.get()) to _types.py
- Export f32x2 from pto.py
- Update ldg/stg ODS descriptions: remove "scalar only", add vector types
and offset/alignment contract
- Update SIMT ISA docs (17-simt.md) with vector type support and offset
semantics (element-level offset, 8-byte alignment for vector<2xf32>)
- Update PTODSL user guide (13-simt-micro-ops.md) with f32x2 example
and offset contract documentation
- Add simt_gm_vector_ldst compile test with i64 dynamic offset
- Add 3 reference examples:
- example_simtvf_vector_add.py: basic f32x2 load/store
- example_simtvf_multi_kernel.py: multi-kernel f32x2 pipeline
- example_int64_stride_vectorize_load.py: stride-based vectorized
load with i64 offset and alignment contract documentation
Co-Authored-By: Claude <noreply@anthropic.com>
- Add VectorType handling to _classify_storage_dtype so pto.ptr(pto.f32x2, "gm") resolves without TypeError. - Add f32x2 to _types.__all__ and expected_public_exports. - Fix example_int64_stride_vectorize_load.py: replace non-existent scalar.convert() with scalar.index_cast(). - Add if __name__ == "__main__" compile entry to all three examples so they serve as self-verifying compile tests. Co-Authored-By: Claude <noreply@anthropic.com>
f16x2 = VectorType.get([2], F16Type.get()) bf16x2 = VectorType.get([2], BF16Type.get()) Backend verifier and lowering already support vector<2xf16> and vector<2xbf16>; this completes the PTODSL type descriptor surface. Co-Authored-By: Claude <noreply@anthropic.com>
Rewrite example_int64_stride_vectorize_load.py to demonstrate the
canonical stride contract:
1. pto.addptr(base, scalar_offset) — advance scalar f32 pointer
2. pto.castptr(scalar_addr, vector_type) — reinterpret as vector ptr
3. pto.ldg(vector_addr, 0) — load with zero offset
This guarantees the stride is applied in f32 units, avoiding
double-multiplication that would occur if the scalar offset were
passed directly to a vector-typed ldg.
Add PTO IR lit test (simt_lowlevel_vector_stride_contract_vpto_llvm.pto)
that verifies the contract lowering through the VPTO backend:
- static stride: addptr + castptr + ldg/stg
- dynamic i64 stride: runtime offset flows into addptr GEP on f32,
castptr adds zero-offset GEP, ldg consumes the result
- no second GEP with runtime index appears in ldg
Co-Authored-By: Claude <noreply@anthropic.com>
1. Fix lit test CHECK: use CHECK-NOT to verify no second runtime-index GEP appears after the addptr GEP (the offset is consumed by addptr; ldg gets constant 0). Remove the brittle check for the castptr GEP which may be optimized away under opaque pointers. 2. Fix example_int64_stride_vectorize_load.py: each thread now writes to dst[tid] instead of racing on dst[0]. 3. Update alignment contract documentation: explicitly state both conditions — base address % 8 == 0 AND scalar_stride % 2 == 0 — are the caller's responsibility. Co-Authored-By: Claude <noreply@anthropic.com>
Capture the addptr GEP for clarity and add CHECK-NOT to ensure no second GEP with a runtime index appears in the ldg path. The castptr GEP (offset 0) may survive lowering as a zero-offset type reinterpretation under opaque pointers, so the ldg operand check uses a wildcard to avoid coupling to the intermediate SSA value. Co-Authored-By: Claude <noreply@anthropic.com>
Capture addptr GEP result as %[[ADDR]]. Add CHECK-NOT for ldg referencing the original base pointer (%arg0), in addition to the existing CHECK-NOT that guards against a second runtime-index GEP. Together these verify: 1. Runtime offset appears only in addptr (f32 GEP). 2. ldg does not fall back to the raw base pointer. Castptr may introduce a zero-offset GEP for type reinterpretation under opaque pointers; the test avoids coupling to that intermediate SSA value while still guarding against the critical contract breaks. Co-Authored-By: Claude <noreply@anthropic.com>
Replace fragile CHECK-NOT guards with explicit capture-and-match of the complete lowering chain: %[[ADDR]] — addptr GEP f32 with runtime i64 index %[[ZERO]] — castptr constant zero %[[VEC_ADDR]] — castptr zero-offset GEP vector<2xf32> ldg consumes %[[VEC_ADDR]] This directly proves the stride contract: runtime offset is consumed by addptr; castptr reinterprets the pointer type with offset 0; ldg sees the final vector pointer. No CHECK-NOT needed. Co-Authored-By: Claude <noreply@anthropic.com>
Remove all lit tests, ptodsl examples, and test modifications from the vector ldg/stg feature branch. Replace them with a single SIMT device test under test/vpto/cases/micro-op/simt/simt-vector-ldg-stg-core/. The test runs 2 lanes, each performing f32x2 vector add via pto.ldg/stg. Verified on CANN SIM: compile + link + run + compare passed. Co-Authored-By: Claude <noreply@anthropic.com>
Match the existing codebase style where other type branches don't have explanatory comments. Co-Authored-By: Claude <noreply@anthropic.com>
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings，因为 review 过程提前失败。 Log Tail |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for packed vector types (vector<2xf16>, vector<2xbf16>, and vector<2xf32>) to the global memory load (pto.ldg) and store (pto.stg) operations, updating documentation, verification, lowering emitters, and the Python DSL, alongside adding a new integration test. The review feedback suggests ensuring that vector types are explicitly verified as non-scalable to prevent downstream compiler crashes, and handling shape mismatches gracefully in the test comparison script to avoid runtime crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+21
to
+28
| if not ok: | ||
| idxs = np.nonzero(~np.isclose(golden, out, rtol=0, atol=0))[0] | ||
| idx = int(idxs[0]) if idxs.size else 0 | ||
| print( | ||
| f"[ERROR] mismatch at idx={idx}, golden={float(golden[idx]):.6f}, out={float(out[idx]):.6f}" | ||
| ) | ||
| if strict: | ||
| sys.exit(2) |
Contributor
There was a problem hiding this comment.
If golden.shape != out.shape, calling np.isclose(golden, out) will raise a ValueError and crash the script. We should handle shape mismatches gracefully by printing a clear error message before attempting element-wise comparison.
Suggested change
| if not ok: | |
| idxs = np.nonzero(~np.isclose(golden, out, rtol=0, atol=0))[0] | |
| idx = int(idxs[0]) if idxs.size else 0 | |
| print( | |
| f"[ERROR] mismatch at idx={idx}, golden={float(golden[idx]):.6f}, out={float(out[idx]):.6f}" | |
| ) | |
| if strict: | |
| sys.exit(2) | |
| if not ok: | |
| if golden.shape != out.shape: | |
| print(f"[ERROR] shape mismatch: golden shape {golden.shape} vs out shape {out.shape}") | |
| else: | |
| idxs = np.nonzero(~np.isclose(golden, out, rtol=0, atol=0))[0] | |
| idx = int(idxs[0]) if idxs.size else 0 | |
| print( | |
| f"[ERROR] mismatch at idx={idx}, golden={float(golden[idx]):.6f}, out={float(out[idx]):.6f}" | |
| ) | |
| if strict: | |
| sys.exit(2) |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
KurrinQu
reviewed
Jul 3, 2026
…te negative test
- Add isPTOPackedFloatVectorType() in PTOTypeUtils, matching verifier:
rank==1, dim==2, elem in {f16, bf16, f32}
- Replace bare VectorType dyn_cast + totalBits checks in both emitters
(10 sites across VPTOLLVMEmitter and VPTOCANN900LLVMEmitter) with the
new helper, so unsupported vector types fall through to failure/default
- Rename simt-vector-ldg-stg-core -> simt-vector-ldg-stg-f32x2-core
- Add simt-vector-ldg-stg-f16x2-core and simt-vector-ldg-stg-bf16x2-core
device tests with golden/compare validation
- Update simt_lowlevel_ldst_policy_negative: remove now-valid vector ldg,
keep GM-pointer-required checks for both ldg and stg
Co-Authored-By: Claude <noreply@anthropic.com>
Zhendong404
suggested changes
Jul 3, 2026
| IntegerType, | ||
| ShapedType, | ||
| Type, | ||
| VectorType, |
Contributor
There was a problem hiding this comment.
这里要和孙可欣对齐下，她的联调分支上应该已经定义了VectorType，你们的定义需要统一
zhangstevenunity
left a comment
zhangstevenunity
left a comment
Collaborator
There was a problem hiding this comment.
Review: GM vectorized pto.ldg / pto.stg
Reviewed the full stack (ODS -> verifier -> LLVM/CANN900 emitters -> ptodsl -> docs -> tests). The design is sound and the implementation is a faithful, minimal extension.
Why it's correct: pto.ldg/pto.stg are lowered through the existing scalar GM intrinsics selected purely by total bit width. vector<2xf16>/vector<2xbf16> (32-bit) reuse the same s32/b32 intrinsic as scalar f32, and vector<2xf32> (64-bit) reuses the same s64/b64 path as scalar f64. The only new work is a width->intrinsic map plus an i32/i64 <-> vector bitcast, mirroring the scalar branches exactly. The element-level offset (GEP strides by the vector's byte size) and 8-byte-alignment-is-caller's-contract story match how scalar f64 already behaves, so no new alignment hazard is introduced. CI is green: build-and-test compiles it and vpto-sim-validation actually compiled+linked+ran+compared the three new device tests on the host SIM.
No blocking issues. A few non-blocking notes (2 inline):
- Helper is looser than the verifier it mirrors (inline on
PTOTypeUtils.cpp). Gemini's!isScalable()fix landed in the verifier, but the sharedisPTOPackedFloatVectorTypehelper - the single gate used across the 10 emitter sites - still doesn't reject scalable vectors. Latent only (verifier blocks it first today), but worth tightening so the helper is self-sufficient. - Negative regression coverage was dropped (inline on the negative lit test). The verifier's new type restriction (rank==1, dim==2, elem in {f16,bf16,f32}) now has no test guarding it. KurrinQu asked about this; the illegal-vector-type cases were removed rather than kept.
compare.py(all three tests): Gemini's shape-mismatch note still applies to the diagnostic branch (~np.isclose/golden != outcan raise on mismatched shapes). Purely cosmetic on the failure path - it still fails-closed with a non-zero exit - so non-blocking.- ptodsl
_classify_storage_dtype: returns"compute"for any scalar-element vector, broader than the backend'sdim-2 {f16,bf16,f32}. The backend still rejects the rest, so this only shifts validation later; fine as-is. - Merge coordination: Zhendong404 flagged a parallel integration branch that also defines
VectorType/ thef*x2descriptors - worth aligning before merge to avoid duplicate/conflicting type descriptors.
| // stg requires GM pointer — UB pointer must fail. | ||
| pto.stg %val, %ub_i32[%c0] l1cache(cache) l2cache(nmfv) : !pto.ptr<i32, ub>, i32 | ||
| // ldg requires GM pointer — UB pointer must fail. | ||
| %bad_ldg = pto.ldg %ub_i32[%c0] l1cache(cache) l2cache(nmfv) : !pto.ptr<i32, ub> -> i32 |
Collaborator
There was a problem hiding this comment.
After this rewrite the only negative coverage is the GM-pointer requirement. The core new constraint - the verifier's type restriction (rank==1, dim==2, element in {f16,bf16,f32}) - now has no regression test anywhere; the earlier vector<4xf32> / vector<3xf16> / vector<2xi32> cases were removed. A single minimal case (e.g. pto.ldg of vector<2xi32> or vector<4xf32> on a GM pointer, // CHECK: currently supports) would guard against a future loosening of the verifier and directly documents the accepted set. KurrinQu raised this thread; suggest keeping at least one illegal-vector-type check rather than dropping them entirely.
mouliangyu
suggested changes
Jul 7, 2026
| return isa<F4E1M2x2Type, F4E2M1x2Type>(t); | ||
| } | ||
|
|
||
| bool mlir::pto::isPTOPackedFloatVectorType(Type t) { |
Contributor
There was a problem hiding this comment.
2xfp8e4m3, 2xfp8e5m2, 2xhif8 以及整形也可以支持下
Co-authored-by: zhangstevenunity <128771452+zhangstevenunity@users.noreply.github.com>
…integer types - Rename isPTOPackedFloatVectorType → isPTOPackedLdgStgVectorType - Extend helper to accept fp8 (f8E4M3FN/f8E5M2), hif8, i8/i16/i32 element types - Gate total bits to 16/32/64 (reject vector<2xi64> at 128 bits) - Use getPTOStorageElemBitWidth for custom PTO type element width - Add 16-bit total path in both emitters: s16/b16 intrinsics, trunc+bitcast - Update verifier to reuse helper, update error message - Update docs to list all supported packed vector types - Add positive lit test and negative verifier test Co-Authored-By: Claude <noreply@anthropic.com>
- Accept !pto.hif8x2 in isPTOPackedLdgStgVectorType as a 16-bit packed type - Add getPTOPackedLdgStgTotalBits helper to compute total bits for both VectorType and non-VectorType packed types (avoids cast<VectorType> crash) - Update both emitters (5 sites each) to use the new helper - Update VPTO.cpp verifier error message, ODS docs, and 17-simt.md to list !pto.hif8x2 and all supported packed types - Remove unparseable vector<2x!pto.hif8> from docs and error messages - Enhance lit test: add !pto.hif8x2 case, fp8 bitcast CHECK lines Co-Authored-By: Claude <noreply@anthropic.com>
- Add simt-vector-ldg-stg-mixed-lowp-int-copy-core SIMT runtime test covering !pto.hif8x2, vector<2xi8>, vector<2xf8E4M3FN>, vector<2xf8E5M2> roundtrip copy (s16/b16 path) — all 4 types pass on NPU simulator - Fix lit test CI failure: remove fragile LLVM dialect type name checks (!llvm.vec<2 x float8e4m3> etc.) that differ across build configs. Replace with stable structural checks (trunc pattern + intrinsic names). Co-Authored-By: Claude <noreply@anthropic.com>
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
6 participants
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
Add packed vector type support for pto.ldg / pto.stg, enabling GM load/store to directly operate on vector<2xf16>, vector<2xbf16>, and vector<2xf32>. Covers the full stack:
ODS definition, IR verification, LLVM/CANN900 lowering, and ptodsl type exposure.
Motivation
pto.ldg / pto.stg currently only support scalar types. In SIMT scenarios, loading/storing two f16/bf16/f32 elements at once (e.g., f32x2 vector add) can replace two scalar
memory operations with a single intrinsic, reducing instruction count and improving bandwidth utilization.
Changes
ODS & IR (include/PTO/IR/VPTOOps.td, lib/PTO/IR/VPTO.cpp)
LLVM Lowering (lib/PTO/Transforms/VPTOLLVMEmitter.cpp)
5 sites extended, following the existing scalar branch pattern:
CANN900 Lowering (lib/PTO/Transforms/VPTOCANN900LLVMEmitter.cpp)
ptodsl (ptodsl/ptodsl/_types.py, ptodsl/ptodsl/pto.py)
Docs
Test
SIM (compile → link → run → compare passed).
Lowering strategy
pto.ldg %ptr[%idx] : !pto.ptr<vector<2xf32>, gm> → vector<2xf32>
↓
%call = llvm.call @llvm.hivm.ldg.cache.s64(%ptr, %c0) : (!llvm.ptr<1>, i32) → i64
%val = llvm.bitcast %call : i64 → vector<2xf32>
Vectors are lowered through the existing scalar intrinsic infrastructure: the total bit width determines the intrinsic suffix (s32/b32 for 32-bit, s64/b64 for 64-bit), and
bitcast bridges the scalar intrinsic interface to the MLIR vector type.