Skip to content

Commit 4828693

Browse files
authored
[Layouts] Fix invertAndCompose when there are identity dimensions (#5468)
I found a counterexample where `invertAndCompose` was giving an incorrect result. Thanks for @lezcano for pointing out a possible solution: if input dim 0 and 4 are identity dimensions, then they are added back to the reduced inverse as dimensions 3 and 4. Them reshaping the result will assign the wrong bases to the wrong dimensions. Use a transpose instead to order the input and output dimensions correctly instead.
1 parent c3f55eb commit 4828693

File tree

2 files changed

+57
-11
lines changed

2 files changed

+57
-11
lines changed

lib/Tools/LinearLayout.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -943,17 +943,10 @@ LinearLayout LinearLayout::invertAndCompose(const LinearLayout &outer) const {
943943
ret *= LinearLayout::identity1D(A.getInDimSize(dim), dim, dim);
944944
}
945945

946-
// Reshape the result
947-
SmallVector<std::pair<StringAttr, int32_t>> inDimsA;
948-
SmallVector<std::pair<StringAttr, int32_t>> inDimsB;
949-
for (auto dim : A.getInDimNames()) {
950-
inDimsA.push_back({dim, A.getInDimSize(dim)});
951-
}
952-
for (auto dim : B.getInDimNames()) {
953-
inDimsB.push_back({dim, B.getInDimSize(dim)});
954-
}
955-
ret = ret.reshapeIns(inDimsB).reshapeOuts(inDimsA);
956-
return ret;
946+
// Reorder the dimensions in the result to match the order expected by the
947+
// current and outer layouts.
948+
return ret.transposeIns(llvm::to_vector(B.getInDimNames()))
949+
.transposeOuts(llvm::to_vector(A.getInDimNames()));
957950
}
958951

959952
LinearLayout LinearLayout::invert() const {

unittest/Tools/LinearLayoutTest.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,59 @@ TEST_F(LinearLayoutTest, InvertAndCompose_BroadcastedDims2) {
513513
EXPECT_EQ(c.compose(b), a.transposeOuts(llvm::to_vector(b.getOutDimNames())));
514514
}
515515

516+
TEST_F(LinearLayoutTest, InvertAndCompose_IdentityInDim) {
517+
SmallVector<StringAttr> outDims = {S("dim0"), S("dim1"), S("dim2"),
518+
S("dim3"), S("dim4"), S("dim5"),
519+
S("dim6"), S("dim7"), S("dim8")};
520+
521+
LinearLayout src({{S("register"),
522+
{
523+
{0, 0, 0, 0, 0, 0, 0, 0, 1},
524+
{0, 0, 0, 0, 0, 0, 0, 1, 0},
525+
}},
526+
{S("lane"),
527+
{
528+
{0, 0, 0, 0, 0, 0, 1, 0, 0},
529+
{0, 0, 0, 0, 0, 1, 0, 0, 0},
530+
{0, 0, 0, 0, 1, 0, 0, 0, 0},
531+
{0, 0, 0, 1, 0, 0, 0, 0, 0},
532+
{0, 0, 1, 0, 0, 0, 0, 0, 0},
533+
}},
534+
{S("warp"),
535+
{
536+
{0, 1, 0, 0, 0, 0, 0, 0, 0},
537+
{1, 0, 0, 0, 0, 0, 0, 0, 0},
538+
}},
539+
{S("block"), {}}},
540+
outDims);
541+
LinearLayout dst({{S("register"),
542+
{
543+
{0, 0, 0, 0, 0, 0, 0, 0, 1},
544+
{0, 0, 0, 0, 0, 0, 0, 1, 0},
545+
}},
546+
{S("lane"),
547+
{
548+
{1, 0, 0, 0, 0, 0, 0, 0, 0},
549+
{0, 1, 0, 0, 0, 0, 0, 0, 0},
550+
{0, 0, 1, 0, 0, 0, 0, 0, 0},
551+
{0, 0, 0, 1, 0, 0, 0, 0, 0},
552+
{0, 0, 0, 0, 1, 0, 0, 0, 0},
553+
}},
554+
{S("warp"),
555+
{
556+
{0, 0, 0, 0, 0, 1, 0, 0, 0},
557+
{0, 0, 0, 0, 0, 0, 1, 0, 0},
558+
}},
559+
{S("block"), {}}},
560+
outDims);
561+
562+
LinearLayout cvt = dst.invertAndCompose(src);
563+
SmallVector<std::pair<StringAttr, int32_t>> k = {
564+
{S("register"), 3}, {S("lane"), 0}, {S("warp"), 2}, {S("block"), 0}};
565+
566+
EXPECT_EQ(dst.apply(k), src.apply(cvt.apply(k)));
567+
}
568+
516569
TEST_F(LinearLayoutTest, NumConsecutiveInOut) {
517570
EXPECT_EQ(
518571
1,

0 commit comments

Comments
 (0)