From b6f49f60d951bf0bb6efcc07435a25456912a6bf Mon Sep 17 00:00:00 2001 From: Ramadhan Gerry Akbar Date: Tue, 2 Jun 2026 14:41:26 +0700 Subject: [PATCH] scheduling: reject reorder() of compressed dimension before its storage parent (fixes #403) --- src/index_notation/transformations.cpp | 51 ++++++++++++++++++++++++++ test/tests-scheduling.cpp | 24 ++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/index_notation/transformations.cpp b/src/index_notation/transformations.cpp index d53ec58c3..f7c51ef42 100644 --- a/src/index_notation/transformations.cpp +++ b/src/index_notation/transformations.cpp @@ -123,6 +123,57 @@ IndexStmt Reorder::apply(IndexStmt stmt, string* reason) const { *reason = "The foralls of reorder pattern: " + util::join(getreplacepattern()) + " were not directly nested."; return IndexStmt(); } + + // Reject reorderings that move a compressed dimension before its storage + // parent. For a tensor stored in CSF/CSR format the position array lookup + // for level L uses the level-(L-1) position variable, so the level-L loop + // must be nested inside the level-(L-1) loop. Placing level-L's index + // variable before level-(L-1)'s index variable in the requested pattern + // silently generates code that reads an undefined variable (issue #403). + struct CheckModeOrderingDeps : public IndexNotationVisitor { + using IndexNotationVisitor::visit; + const std::vector& pattern; + std::string* reason; + + CheckModeOrderingDeps(const std::vector& p, std::string* r) + : pattern(p), reason(r) {} + + void visit(const AccessNode* node) { + const auto& modeOrdering = node->tensorVar.getFormat().getModeOrdering(); + const auto& modeFormats = node->tensorVar.getFormat().getModeFormats(); + for (size_t m = 1; m < modeOrdering.size(); ++m) { + int prevMode = modeOrdering[m - 1]; + int curMode = modeOrdering[m]; + // Dense levels are accessed by coordinate and do not carry a + // positional dependency, so no constraint applies to this pair. + if (modeFormats[prevMode] == ModeFormat::Dense) continue; + + IndexVar prevVar = node->indexVars[prevMode]; + IndexVar curVar = node->indexVars[curMode]; + + auto prevIt = std::find(pattern.begin(), pattern.end(), prevVar); + auto curIt = std::find(pattern.begin(), pattern.end(), curVar); + if (prevIt == pattern.end() || curIt == pattern.end()) continue; + + if (curIt < prevIt) { + *reason = "Cannot reorder " + util::toString(curVar) + + " before " + util::toString(prevVar) + + ": in tensor " + node->tensorVar.getName() + + ", dimension " + util::toString(curVar) + + " is at a compressed level whose position array is" + + " indexed by " + util::toString(prevVar) + "'s iterator."; + return; + } + } + } + }; + + CheckModeOrderingDeps checker(getreplacepattern(), reason); + stmt.accept(&checker); + if (!reason->empty()) { + return IndexStmt(); + } + return ForAllReplace(currentOrdering, getreplacepattern()).apply(stmt, reason); } diff --git a/test/tests-scheduling.cpp b/test/tests-scheduling.cpp index ee564577b..a76ccc199 100644 --- a/test/tests-scheduling.cpp +++ b/test/tests-scheduling.cpp @@ -998,6 +998,30 @@ TEST(scheduling_eval_test, indexVarReorder) { ASSERT_TENSOR_EQ(expected, a); } +// Regression test for issue #403: reordering a compressed dimension before its +// storage parent must be rejected with a clear error rather than silently +// emitting code that reads an undefined position variable. +TEST(scheduling, reorder_invalid_sparse_dependency) { + // SpTV: y(j) = A(i,j,k) * x(k), A stored as sss (order i,j,k). + // j's compressed level is indexed by i's position variable, so reorder(j,i,k) + // is invalid — j cannot be iterated before i. + Tensor y("y", {4}, Sparse); + Tensor A("A", {4, 4, 4}, {Compressed, Compressed, Compressed}); + Tensor x("x", {4}, Sparse); + + IndexVar ii("i"), jj("j"), kk("k"); + y(jj) = A(ii, jj, kk) * x(kk); + IndexStmt stmt = y.getAssignment().concretize(); + + std::string reason; + IndexStmt result = Reorder({jj, ii, kk}).apply(stmt, &reason); + + ASSERT_FALSE(result.defined()) + << "Expected reorder(j,i,k) to fail for sss tensor, but it succeeded"; + ASSERT_FALSE(reason.empty()) + << "Expected a non-empty error reason for invalid sparse reordering"; +} + TEST(scheduling, divide) { auto dim = 256; float sparsity = 0.1;