Skip to content

Conversation

@banach-space
Copy link
Contributor

The only way to test isProjectedPermutation is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.

@llvmbot
Copy link
Member

llvmbot commented Nov 4, 2024

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

The only way to test isProjectedPermutation is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.


Full diff: https://github.com/llvm/llvm-project/pull/114775.diff

1 Files Affected:

  • (modified) mlir/unittests/IR/AffineMapTest.cpp (+41)
diff --git a/mlir/unittests/IR/AffineMapTest.cpp b/mlir/unittests/IR/AffineMapTest.cpp
index 081afadd632f1b..2c96c69a69d203 100644
--- a/mlir/unittests/IR/AffineMapTest.cpp
+++ b/mlir/unittests/IR/AffineMapTest.cpp
@@ -21,3 +21,44 @@ TEST(AffineMapTest, inferMapFromAffineExprs) {
   map.replace(replacements);
   EXPECT_EQ(map, map);
 }
+
+TEST(AffineMapTest, isProjectedPermutation) {
+  MLIRContext ctx;
+  OpBuilder b(&ctx);
+
+  // 1. Empty map - a projected permutation.
+  AffineMap map1 = b.getEmptyAffineMap();
+  EXPECT_TRUE(map1.isProjectedPermutation());
+
+  // 2. Contains a symbol - not a projected permutation.
+  AffineMap map2 = AffineMap::get(0, 1, &ctx);
+  EXPECT_FALSE(map2.isProjectedPermutation());
+
+  // 3. The result map is {0} - since zero results are _allowed_, this _is_ a
+  // projected permutation.
+  auto zero = b.getAffineConstantExpr(0);
+  AffineMap map3 = AffineMap::get(1, 0, {zero}, &ctx);
+  EXPECT_TRUE(map3.isProjectedPermutation(/*allowZeroInResults=*/true));
+
+  // 4. The result map is {0} - since zero results are _not allowed_, this _is not_
+  // a projected permutation.
+  AffineMap map4 = AffineMap::get(1, 0, {zero}, &ctx);
+  EXPECT_FALSE(map4.isProjectedPermutation(/*allowZeroInResults=*/false));
+
+  // 5. The number of results > inputs, not a projected permutation.
+  AffineMap map5 = AffineMap::get(1, 0, {zero, zero}, &ctx);
+  EXPECT_FALSE(map5.isProjectedPermutation(/*allowZeroInResults=*/true));
+
+  // 6. A constant result that's not a {0} - not a projected permutation.
+  auto one = b.getAffineConstantExpr(1);
+  AffineMap map6 = AffineMap::get(1, 0, {one}, &ctx);
+  EXPECT_FALSE(map6.isProjectedPermutation(/*allowZeroInResults=*/true));
+
+  // 7. Not a dim expression - not a projected permutation.
+  auto d0 = b.getAffineDimExpr(0);
+  auto d1 = b.getAffineDimExpr(1);
+
+  auto sum = d0 + d1;
+  AffineMap map7 = AffineMap::get(2, 0, {sum}, &ctx);
+  EXPECT_FALSE(map7.isProjectedPermutation());
+}

@github-actions
Copy link

github-actions bot commented Nov 4, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

The only way to test `isProjectedPermutation` is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.
@banach-space banach-space force-pushed the andrzej/add_unit_tests_for_map branch from dc8ada1 to 9f03639 Compare November 4, 2024 15:17
Copy link
Contributor

@javedabsar1 javedabsar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. ProjectedPermutation is used in many places and these are very instructive tests.

MLIRContext ctx;
OpBuilder b(&ctx);

// 1. Empty map - a projected permutation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its a 'projected permutation' test and TRUE, FALSE tells the expected so no need to repeat 'a projected permutation'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, thanks!

// 7. Not a dim expression - not a projected permutation.
auto d0 = b.getAffineDimExpr(0);
auto d1 = b.getAffineDimExpr(1);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some tougher tests
(d0,d1, d2, d3,d4,d5) ->(d5,d3,d0,d1,d2,d4)
(d0,d1, d2, d3,d4,d5) ->(d5,d3,d0+d1,d2,d4)
(d0,d1, d2, d3,d4,d5) ->(d5,d3,d2,d4)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, thanks, about to add these.

@banach-space banach-space force-pushed the andrzej/add_unit_tests_for_map branch from f06eb81 to ef83b59 Compare November 5, 2024 13:53
Copy link
Contributor

@javedabsar1 javedabsar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great now. Thanks.

@banach-space banach-space merged commit 8b7af60 into llvm:main Nov 5, 2024
8 checks passed
@banach-space banach-space deleted the andrzej/add_unit_tests_for_map branch November 5, 2024 16:54
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
The only way to test `isProjectedPermutation` is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.
banach-space added a commit to banach-space/llvm-project that referenced this pull request Nov 20, 2024
The only way to test `getInversePermutation` is through unit tests. The
concept of "inverse permutations" is tricky to document and these tests
are a good source documentation of the expected/intended behavoiur.
Hence these additional unit tests.

This is a follon-on llvm#114775 for in which I added tests for
`isProjectedPermutation`.
banach-space added a commit that referenced this pull request Nov 22, 2024
The only way to test `getInversePermutation` is through unit tests. The
concept of "inverse permutations" is tricky to document and these tests
are a good source documentation of the expected/intended behavoiur.
Hence these additional unit tests.

This is a follow-on of #114775 in which I added tests for
`isProjectedPermutation`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants