[Mlir-commits] [mlir] 1e15d79 - [mlir][sparse] minor cleanup of merger unit test

Aart Bik llvmlistbot at llvm.org
Mon Aug 7 14:54:25 PDT 2023


Author: Aart Bik
Date: 2023-08-07T14:54:16-07:00
New Revision: 1e15d7911322c63fdb3ecdfe8fe081db190cd375

URL: https://github.com/llvm/llvm-project/commit/1e15d7911322c63fdb3ecdfe8fe081db190cd375
DIFF: https://github.com/llvm/llvm-project/commit/1e15d7911322c63fdb3ecdfe8fe081db190cd375.diff

LOG: [mlir][sparse] minor cleanup of merger unit test

Removed some of the warning supression needed for
the multi-arg macro logic by making number of arguments
the same everywhere. Also removes some verbose comments
and obvious TODOs.

Reviewed By: wrengr

Differential Revision: https://reviews.llvm.org/D157327

Added: 
    

Modified: 
    mlir/unittests/Dialect/SparseTensor/MergerTest.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/unittests/Dialect/SparseTensor/MergerTest.cpp b/mlir/unittests/Dialect/SparseTensor/MergerTest.cpp
index 00760c02bb63e9..561753f631c16f 100644
--- a/mlir/unittests/Dialect/SparseTensor/MergerTest.cpp
+++ b/mlir/unittests/Dialect/SparseTensor/MergerTest.cpp
@@ -2,20 +2,12 @@
 #include "llvm/Support/Compiler.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+
 #include <memory>
 
 using namespace mlir;
 using namespace mlir::sparse_tensor;
 
-// Silence 'warning C4002: 'too many arguments for function-liked macro
-//                          invocation'
-// as MSVC handles ##__VA_ARGS__ 
diff erently as gcc/clang
-
-#if defined(_MSC_VER) && !defined(__clang__)
-#pragma warning(push)
-#pragma warning(disable : 4002)
-#endif
-
 namespace {
 
 ///
@@ -38,45 +30,44 @@ namespace {
   DO(cmpf, TensorExp::Kind::kCmpF)                                             \
   DO(cmpi, TensorExp::Kind::kCmpI)
 
-// TODO: Disjunctive binary operations that need special handling are not
-// included, e.g., Division are not tested (for now) as it need a constant
-// non-zero dividend.
-// ##__VA_ARGS__ handles cases when __VA_ARGS__ is empty.
-#define FOREVERY_COMMON_DISJ_BINOP(TEST, ...)                                  \
-  TEST(addf, ##__VA_ARGS__)                                                    \
-  TEST(addc, ##__VA_ARGS__)                                                    \
-  TEST(addi, ##__VA_ARGS__)                                                    \
-  TEST(xori, ##__VA_ARGS__)                                                    \
-  TEST(ori, ##__VA_ARGS__)
-
-// TODO: Conjunctive binary operations that need special handling are not
-// included, e.g., substraction yields a 
diff erent pattern as it is mapped to
-// negate operation.
-#define FOREVERY_COMMON_CONJ_BINOP(TEST, ...)                                  \
-  TEST(mulf, ##__VA_ARGS__)                                                    \
-  TEST(mulc, ##__VA_ARGS__)                                                    \
-  TEST(muli, ##__VA_ARGS__)                                                    \
-  TEST(andi, ##__VA_ARGS__)
+#define FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, EXTRA)                          \
+  TEST(addf, EXTRA)                                                            \
+  TEST(addc, EXTRA)                                                            \
+  TEST(addi, EXTRA)                                                            \
+  TEST(xori, EXTRA)                                                            \
+  TEST(ori, EXTRA)
+
+#define FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, EXTRA)                          \
+  TEST(mulf, EXTRA)                                                            \
+  TEST(mulc, EXTRA)                                                            \
+  TEST(muli, EXTRA)                                                            \
+  TEST(andi, EXTRA)
+
+#define FOREVERY_COMMON_DISJ_BINOP(TEST)                                       \
+  FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, "")
+
+#define FOREVERY_COMMON_CONJ_BINOP(TEST)                                       \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, "")
 
 #define FOREVERY_PAIR_OF_COMMON_CONJ_DISJ_BINOP(TEST)                          \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, addf)                                       \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, addc)                                       \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, addi)                                       \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, xori)                                       \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, ori)
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, addf)                                 \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, addc)                                 \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, addi)                                 \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, xori)                                 \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, ori)
 
 #define FOREVERY_PAIR_OF_COMMON_CONJ_CONJ_BINOP(TEST)                          \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, mulf)                                       \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, mulc)                                       \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, muli)                                       \
-  FOREVERY_COMMON_CONJ_BINOP(TEST, andi)
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, mulf)                                 \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, mulc)                                 \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, muli)                                 \
+  FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, andi)
 
 #define FOREVERY_PAIR_OF_COMMON_DISJ_DISJ_BINOP(TEST)                          \
-  FOREVERY_COMMON_DISJ_BINOP(TEST, addf)                                       \
-  FOREVERY_COMMON_DISJ_BINOP(TEST, addc)                                       \
-  FOREVERY_COMMON_DISJ_BINOP(TEST, addi)                                       \
-  FOREVERY_COMMON_DISJ_BINOP(TEST, ori)                                        \
-  FOREVERY_COMMON_DISJ_BINOP(TEST, xori)
+  FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, addf)                                 \
+  FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, addc)                                 \
+  FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, addi)                                 \
+  FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, ori)                                  \
+  FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, xori)
 
 ///
 /// Helper classes/functions for testing Merger.
@@ -87,9 +78,6 @@ struct Pattern;
 /// Since the patterns we need are rather small and short-lived, we use
 /// `Pattern const&` for "pointers" to patterns, rather than using
 /// something more elaborate like `std::shared_ptr<Pattern> const&`.
-/// (But since we use a typedef rather than spelling it out everywhere,
-/// that's easy enough to swap out if we need something more elaborate
-/// in the future.)
 using PatternRef = const Pattern &;
 struct Pattern {
   struct Children {
@@ -109,7 +97,7 @@ struct Pattern {
   };
 
   /// Constructors.
-  /// Rather than using these, please use the readable helper constructor
+  /// Rather than using these, please use the readable builder
   /// functions below to make tests more readable.
   Pattern() : kind(TensorExp::Kind::kSynZero) {}
   Pattern(TensorId tid) : kind(TensorExp::Kind::kTensor), tid(tid) {}
@@ -505,7 +493,7 @@ FOREVERY_PAIR_OF_COMMON_CONJ_CONJ_BINOP(IMPL_MERGER_TEST_CONJ_CONJ_SPARSE_OUT)
 ///   lat( i_00 / tensor_0 )
 ///   lat( i_01 / tensor_1 )
 /// }
-#define IMPL_MERGER_TEST_DISJ(OP)                                              \
+#define IMPL_MERGER_TEST_DISJ(OP, UNUSED)                                      \
   TEST_F(MergerTest3T1L, vector_##OP) {                                        \
     const auto e = OP##Expr(tensor(0), tensor(1));                             \
     const auto l0 = lid(0);                                                    \
@@ -539,7 +527,7 @@ FOREVERY_COMMON_DISJ_BINOP(IMPL_MERGER_TEST_DISJ)
 /// {
 ///   lat( i_00 i_01 / (tensor_0 * tensor_1) )
 /// }
-#define IMPL_MERGER_TEST_CONJ(OP)                                              \
+#define IMPL_MERGER_TEST_CONJ(OP, UNUSED)                                      \
   TEST_F(MergerTest3T1L, vector_##OP) {                                        \
     const auto e = OP##Expr(tensor(0), tensor(1));                             \
     const auto l0 = lid(0);                                                    \
@@ -708,7 +696,7 @@ FOREVERY_PAIR_OF_COMMON_CONJ_CONJ_BINOP(IMPL_MERGER_TEST_CONJ_CONJ)
 ///
 /// lat( i_00 / sparse_tensor_0 ) should be opted out as it only has dense 
diff 
 /// with lat( i_00 i_01 / (sparse_tensor_0 + dense_tensor_1) ).
-#define IMPL_MERGER_TEST_OPTIMIZED_DISJ(OP)                                    \
+#define IMPL_MERGER_TEST_OPTIMIZED_DISJ(OP, UNUSED)                            \
   TEST_F(MergerTest3T1LD, vector_opted_##OP) {                                 \
     const auto e = OP##Expr(tensor(0), tensor(1));                             \
     const auto l0 = lid(0);                                                    \
@@ -746,7 +734,7 @@ FOREVERY_COMMON_DISJ_BINOP(IMPL_MERGER_TEST_OPTIMIZED_DISJ)
 ///   lat( i_00 / (sparse_tensor_0 * dense_tensor_1) )
 /// }
 /// since i_01 is a dense dimension.
-#define IMPL_MERGER_TEST_OPTIMIZED_CONJ(OP)                                    \
+#define IMPL_MERGER_TEST_OPTIMIZED_CONJ(OP, UNUSED)                            \
   TEST_F(MergerTest3T1LD, vector_opted_##OP) {                                 \
     const auto e = OP##Expr(tensor(0), tensor(1));                             \
     const auto l0 = lid(0);                                                    \
@@ -841,10 +829,3 @@ TEST_F(MergerTest3T1LD, vector_cmp) {
 }
 
 #undef IMPL_MERGER_TEST_OPTIMIZED_CONJ
-
-// TODO: mult-dim tests
-
-// restore warning status
-#if defined(_MSC_VER) && !defined(__clang__)
-#pragma warning(pop)
-#endif


        


More information about the Mlir-commits mailing list