Skip to content

Commit 1e15d79

Browse files
aartbikaartbik
committed
[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
1 parent 769333a commit 1e15d79

File tree

1 file changed

+38
-57
lines changed

1 file changed

+38
-57
lines changed

‎mlir/unittests/Dialect/SparseTensor/MergerTest.cpp

Lines changed: 38 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,12 @@
22
#include "llvm/Support/Compiler.h"
33
#include "gmock/gmock.h"
44
#include "gtest/gtest.h"
5+
56
#include <memory>
67

78
using namespace mlir;
89
using namespace mlir::sparse_tensor;
910

10-
// Silence 'warning C4002: 'too many arguments for function-liked macro
11-
// invocation'
12-
// as MSVC handles ##__VA_ARGS__ differently as gcc/clang
13-
14-
#if defined(_MSC_VER) && !defined(__clang__)
15-
#pragma warning(push)
16-
#pragma warning(disable : 4002)
17-
#endif
18-
1911
namespace {
2012

2113
///
@@ -38,45 +30,44 @@ namespace {
3830
DO(cmpf, TensorExp::Kind::kCmpF) \
3931
DO(cmpi, TensorExp::Kind::kCmpI)
4032

41-
// TODO: Disjunctive binary operations that need special handling are not
42-
// included, e.g., Division are not tested (for now) as it need a constant
43-
// non-zero dividend.
44-
// ##__VA_ARGS__ handles cases when __VA_ARGS__ is empty.
45-
#define FOREVERY_COMMON_DISJ_BINOP(TEST, ...) \
46-
TEST(addf, ##__VA_ARGS__) \
47-
TEST(addc, ##__VA_ARGS__) \
48-
TEST(addi, ##__VA_ARGS__) \
49-
TEST(xori, ##__VA_ARGS__) \
50-
TEST(ori, ##__VA_ARGS__)
51-
52-
// TODO: Conjunctive binary operations that need special handling are not
53-
// included, e.g., substraction yields a different pattern as it is mapped to
54-
// negate operation.
55-
#define FOREVERY_COMMON_CONJ_BINOP(TEST, ...) \
56-
TEST(mulf, ##__VA_ARGS__) \
57-
TEST(mulc, ##__VA_ARGS__) \
58-
TEST(muli, ##__VA_ARGS__) \
59-
TEST(andi, ##__VA_ARGS__)
33+
#define FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, EXTRA) \
34+
TEST(addf, EXTRA) \
35+
TEST(addc, EXTRA) \
36+
TEST(addi, EXTRA) \
37+
TEST(xori, EXTRA) \
38+
TEST(ori, EXTRA)
39+
40+
#define FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, EXTRA) \
41+
TEST(mulf, EXTRA) \
42+
TEST(mulc, EXTRA) \
43+
TEST(muli, EXTRA) \
44+
TEST(andi, EXTRA)
45+
46+
#define FOREVERY_COMMON_DISJ_BINOP(TEST) \
47+
FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, "")
48+
49+
#define FOREVERY_COMMON_CONJ_BINOP(TEST) \
50+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, "")
6051

6152
#define FOREVERY_PAIR_OF_COMMON_CONJ_DISJ_BINOP(TEST) \
62-
FOREVERY_COMMON_CONJ_BINOP(TEST, addf) \
63-
FOREVERY_COMMON_CONJ_BINOP(TEST, addc) \
64-
FOREVERY_COMMON_CONJ_BINOP(TEST, addi) \
65-
FOREVERY_COMMON_CONJ_BINOP(TEST, xori) \
66-
FOREVERY_COMMON_CONJ_BINOP(TEST, ori)
53+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, addf) \
54+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, addc) \
55+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, addi) \
56+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, xori) \
57+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, ori)
6758

6859
#define FOREVERY_PAIR_OF_COMMON_CONJ_CONJ_BINOP(TEST) \
69-
FOREVERY_COMMON_CONJ_BINOP(TEST, mulf) \
70-
FOREVERY_COMMON_CONJ_BINOP(TEST, mulc) \
71-
FOREVERY_COMMON_CONJ_BINOP(TEST, muli) \
72-
FOREVERY_COMMON_CONJ_BINOP(TEST, andi)
60+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, mulf) \
61+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, mulc) \
62+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, muli) \
63+
FOREVERY_COMMON_CONJ_BINOP_EXTRA(TEST, andi)
7364

7465
#define FOREVERY_PAIR_OF_COMMON_DISJ_DISJ_BINOP(TEST) \
75-
FOREVERY_COMMON_DISJ_BINOP(TEST, addf) \
76-
FOREVERY_COMMON_DISJ_BINOP(TEST, addc) \
77-
FOREVERY_COMMON_DISJ_BINOP(TEST, addi) \
78-
FOREVERY_COMMON_DISJ_BINOP(TEST, ori) \
79-
FOREVERY_COMMON_DISJ_BINOP(TEST, xori)
66+
FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, addf) \
67+
FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, addc) \
68+
FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, addi) \
69+
FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, ori) \
70+
FOREVERY_COMMON_DISJ_BINOP_EXTRA(TEST, xori)
8071

8172
///
8273
/// Helper classes/functions for testing Merger.
@@ -87,9 +78,6 @@ struct Pattern;
8778
/// Since the patterns we need are rather small and short-lived, we use
8879
/// `Pattern const&` for "pointers" to patterns, rather than using
8980
/// something more elaborate like `std::shared_ptr<Pattern> const&`.
90-
/// (But since we use a typedef rather than spelling it out everywhere,
91-
/// that's easy enough to swap out if we need something more elaborate
92-
/// in the future.)
9381
using PatternRef = const Pattern &;
9482
struct Pattern {
9583
struct Children {
@@ -109,7 +97,7 @@ struct Pattern {
10997
};
11098

11199
/// Constructors.
112-
/// Rather than using these, please use the readable helper constructor
100+
/// Rather than using these, please use the readable builder
113101
/// functions below to make tests more readable.
114102
Pattern() : kind(TensorExp::Kind::kSynZero) {}
115103
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)
505493
/// lat( i_00 / tensor_0 )
506494
/// lat( i_01 / tensor_1 )
507495
/// }
508-
#define IMPL_MERGER_TEST_DISJ(OP) \
496+
#define IMPL_MERGER_TEST_DISJ(OP, UNUSED) \
509497
TEST_F(MergerTest3T1L, vector_##OP) { \
510498
const auto e = OP##Expr(tensor(0), tensor(1)); \
511499
const auto l0 = lid(0); \
@@ -539,7 +527,7 @@ FOREVERY_COMMON_DISJ_BINOP(IMPL_MERGER_TEST_DISJ)
539527
/// {
540528
/// lat( i_00 i_01 / (tensor_0 * tensor_1) )
541529
/// }
542-
#define IMPL_MERGER_TEST_CONJ(OP) \
530+
#define IMPL_MERGER_TEST_CONJ(OP, UNUSED) \
543531
TEST_F(MergerTest3T1L, vector_##OP) { \
544532
const auto e = OP##Expr(tensor(0), tensor(1)); \
545533
const auto l0 = lid(0); \
@@ -708,7 +696,7 @@ FOREVERY_PAIR_OF_COMMON_CONJ_CONJ_BINOP(IMPL_MERGER_TEST_CONJ_CONJ)
708696
///
709697
/// lat( i_00 / sparse_tensor_0 ) should be opted out as it only has dense diff
710698
/// with lat( i_00 i_01 / (sparse_tensor_0 + dense_tensor_1) ).
711-
#define IMPL_MERGER_TEST_OPTIMIZED_DISJ(OP) \
699+
#define IMPL_MERGER_TEST_OPTIMIZED_DISJ(OP, UNUSED) \
712700
TEST_F(MergerTest3T1LD, vector_opted_##OP) { \
713701
const auto e = OP##Expr(tensor(0), tensor(1)); \
714702
const auto l0 = lid(0); \
@@ -746,7 +734,7 @@ FOREVERY_COMMON_DISJ_BINOP(IMPL_MERGER_TEST_OPTIMIZED_DISJ)
746734
/// lat( i_00 / (sparse_tensor_0 * dense_tensor_1) )
747735
/// }
748736
/// since i_01 is a dense dimension.
749-
#define IMPL_MERGER_TEST_OPTIMIZED_CONJ(OP) \
737+
#define IMPL_MERGER_TEST_OPTIMIZED_CONJ(OP, UNUSED) \
750738
TEST_F(MergerTest3T1LD, vector_opted_##OP) { \
751739
const auto e = OP##Expr(tensor(0), tensor(1)); \
752740
const auto l0 = lid(0); \
@@ -841,10 +829,3 @@ TEST_F(MergerTest3T1LD, vector_cmp) {
841829
}
842830

843831
#undef IMPL_MERGER_TEST_OPTIMIZED_CONJ
844-
845-
// TODO: mult-dim tests
846-
847-
// restore warning status
848-
#if defined(_MSC_VER) && !defined(__clang__)
849-
#pragma warning(pop)
850-
#endif

0 commit comments

Comments
 (0)