[TritonToLinalg](fix) lower zero-stride block pointer loads - #1508
[TritonToLinalg](fix) lower zero-stride block pointer loads#1508zhanwei33 wants to merge 2 commits into
Conversation
|
🔍 OpenCodeReview found 1 issue(s) in this PR.
|
| const bool zeroStrideBroadcast = hasStaticZeroStride(strides); | ||
| int64_t lastStride = 0; | ||
| if (!zeroStrideBroadcast) { |
There was a problem hiding this comment.
[other · low]
lastStride is declared at function scope with initial value 0, but is only meaningfully assigned inside the if (!zeroStrideBroadcast) block (line 856). When zeroStrideBroadcast=true, the code path reaches line 1022 (LLVM_DEBUG printing last_stride), where lastStride remains 0 rather than reflecting the actual last stride value. Since hasStaticZeroStride only checks if any stride is 0, the last stride may well be non-zero, making the debug output misleading.
Suggestion: move lastStride inside the if (!zeroStrideBroadcast) block to scope it correctly, and for the debug output at line 1022 either extract the last stride on-the-fly from strides.back() or conditionally skip printing it for the broadcast case.
Suggestion:
| const bool zeroStrideBroadcast = hasStaticZeroStride(strides); | |
| int64_t lastStride = 0; | |
| if (!zeroStrideBroadcast) { | |
| const bool zeroStrideBroadcast = hasStaticZeroStride(strides); | |
| if (!zeroStrideBroadcast) { | |
| int64_t lastStride = 0; | |
| // ... order check, stride check ... | |
| APInt lastStrideC; | |
| if (!matchPattern(strides.back(), m_ConstantInt(&lastStrideC))) | |
| return failure(); | |
| lastStride = std::abs(lastStrideC.getSExtValue()); | |
| // ... |
Summary
tt.make_tensor_ptrlogical shape and offset when a stride is staticallyzero, instead of reconstructing logical coordinates by dividing a physical
offset by stride.
the generic scalar-loop fallback for ordinary targets/modes, and the
existing A5 indirect-load form for
compile_on_910_95 && force_simt_template.negative-offset, and nonzero-stride cases.
Root cause
Block-pointer analysis multiplies logical offsets by strides. A zero stride
therefore erases that logical coordinate from the physical offset. The old
boundary helper attempted to recover it with
physical_offset / stride,which reaches
0 / 0and crashes the conversion.Scope
Only statically known zero-stride
tt.make_tensor_ptrloads are changed.Stores, dynamically zero strides, non-MTP accesses, and statically nonzero
strides retain their existing paths.
Alternative
#1509 contains the boundary-only alternative: it fixes the conversion crash
but deliberately retains the downstream
strided<[0, ...]>representation.This PR includes the downstream-safe lowering. Do not merge both PRs.
Validation
pre-commit run --fileson all six changed source/test filestriton-opt --pass-pipeline='builtin.module(triton-to-unstructure{compile-on-910-95=false force-simt-template=true},triton-to-linalg{compile-on-910-95=false enable-nd2nz-on-vector=false enable-select-analysis=true global-kernel=false named-ops=true})' --split-input-file third_party/ascend/unittest/Conversion/General/TritonToLinalg/zero_stride_block_ptr_load.mlir | FileCheck third_party/ascend/unittest/Conversion/General/TritonToLinalg/zero_stride_block_ptr_load.mlirThe focused test passes; it emits only pre-existing location warnings from
linalg.yield.