[ARITH] RewriteSimplifier: min/max, logical, select #2768

tqchen · 2019-03-10T21:21:35Z

This is followup of #2722

Added support for min/max, logical, select
Added test cases to cover the rules

tqchen · 2019-03-10T21:22:00Z

cc @sgrechanik-h @icemelon9 @merrymercy @kazum @grwlf @ZihengJiang @wweic @junrushao1994 @MarisaKirisame please help review the PR

src/arithmetic/rewrite_simplify.cc

sgrechanik-h · 2019-03-11T14:17:00Z

src/arithmetic/rewrite_simplify.cc

+    TVM_TRY_REWRITE(min(min(y, x), min(z, x)), min(min(y, z), x));
+
+    TVM_TRY_REWRITE(min(y + x, z + x), min(y, z) + x);
+    TVM_TRY_REWRITE(min(y + x, x + z), min(y, z) + x);


I guess something similar but for - might be needed here.

src/arithmetic/rewrite_simplify.cc

sgrechanik-h · 2019-03-11T16:08:03Z

(((i0 % 2) != 1) || ((i0 % 2) == 0)) gets rewritten into ((i0 % 2) != 0), don't know what rule causes it. By the way, it would be great to have some rule debugging tool, may be something like printing out every transformation step if some compilation flag is on.

Upd: seems like these ones

  TVM_TRY_REWRITE(x == c1 || x != c2, x != c1 || c1 == c2);
  TVM_TRY_REWRITE(x != c2 || x == c1, x != c1 || c1 == c2);

src/arithmetic/rewrite_simplify.cc

tqchen · 2019-03-12T03:20:12Z

Thanks @sgrechanik-h I have updated per your comment

src/arithmetic/rewrite_simplify.cc

kazum · 2019-03-12T17:47:51Z

src/arithmetic/rewrite_simplify.cc

+      int64_t c1val = c1.Eval()->value;
+      int64_t c2val = c2.Eval()->value;
+      if (c2val % c1val == 0) {
+        if (c2val / c1val > 0) {


I think > should be >= here since c2val can be zero.

kazum · 2019-03-12T17:49:34Z

src/arithmetic/rewrite_simplify.cc

+                       c1.Eval()->value + 1 == c2.Eval()->value);
+
+    TVM_TRY_REWRITE(max(min(x, y), max(x, y)), max(x, y));
+    TVM_TRY_REWRITE(max(min(x, y), max(y, x)), max(x, y));


Need the following ones too?

TVM_TRY_REWRITE(max(max(x, y), min(x, y)), max(x, y)); TVM_TRY_REWRITE(max(max(x, y), min(y, x)), max(x, y));

kazum · 2019-03-12T17:50:08Z

src/arithmetic/rewrite_simplify.cc

+      int64_t c1val = c1.Eval()->value;
+      int64_t c2val = c2.Eval()->value;
+      if (c2val % c1val == 0) {
+        if (c2val / c1val > 0) {


> should be >= here.

kazum · 2019-03-12T17:55:25Z

tests/python/unittest/test_arith_rewrite_simplify.py

+    ck.analyzer.update(x, tvm.arith.ConstIntBound(0, 1000))
+    ck.verify(tvm.min((x + 3) / 4 * 4, tvm.max(x, 4)), tvm.max(x, 4))
+    ck.verify(tvm.min(x, (x + 3) / 4 * 4), x)
+    ck.verify(tvm.min(tvm.max(x, 4), (x + 3) / 4 * 4), tvm.max(x, 4))


I think the boundary of x should be reset after the check of this rule.

wweic · 2019-03-12T18:37:28Z

src/arithmetic/rewrite_simplify.cc

+    TVM_TRY_REWRITE(min(x, min(x, y)), min(x, y));
+    TVM_TRY_REWRITE(min(y, min(x, y)), min(x, y));
+
+    TVM_TRY_REWRITE(min(min(min(x, y), z), y), min(min(x, y), z));


Also?

TVM_TRY_REWRITE(min(min(min(x, y), z), x), min(min(x, y), z)); TVM_TRY_REWRITE(min(min(min(x, y), z), z), min(min(x, y), z));

wweic · 2019-03-12T18:44:45Z

src/arithmetic/rewrite_simplify.cc

+    TVM_TRY_REWRITE(min(y, min(x, y)), min(x, y));
+
+    TVM_TRY_REWRITE(min(min(min(x, y), z), y), min(min(x, y), z));
+    TVM_TRY_REWRITE(min(min(min(min(x, y), z), s1), y), min(min(min(x, y), z), s1));


Add these?

TVM_TRY_REWRITE(min(min(min(min(x, y), z), s1), x), min(min(min(x, y), z), s1)); TVM_TRY_REWRITE(min(min(min(min(x, y), z), s1), z), min(min(min(x, y), z), s1)); TVM_TRY_REWRITE(min(min(min(min(x, y), z), s1), s1), min(min(min(x, y), z), s1));

wweic · 2019-03-12T18:45:41Z

src/arithmetic/rewrite_simplify.cc

+
+    TVM_TRY_REWRITE(min(min(min(x, y), z), y), min(min(x, y), z));
+    TVM_TRY_REWRITE(min(min(min(min(x, y), z), s1), y), min(min(min(x, y), z), s1));
+    TVM_TRY_REWRITE(min(min(min(min(min(x, y), z), s1), s2), y),


TVM_TRY_REWRITE(min(min(min(min(min(x, y), z), s1), s2), x), min(min(min(min(x, y), z), s1), s2)); TVM_TRY_REWRITE(min(min(min(min(min(x, y), z), s1), s2), z), min(min(min(min(x, y), z), s1), s2)); TVM_TRY_REWRITE(min(min(min(min(min(x, y), z), s1), s2), s1), min(min(min(min(x, y), z), s1), s2)); TVM_TRY_REWRITE(min(min(min(min(min(x, y), z), s1), s2), s2), min(min(min(min(x, y), z), s1), s2));

wweic · 2019-03-12T20:12:20Z

src/arithmetic/rewrite_simplify.cc

+  TVM_TRY_REWRITE(y < x && y <= x, cfalse);
+
+  TVM_TRY_REWRITE_IF(x < c1 && c2 < x, cfalse,
+                     c2.Eval()->value + 1 >= c1.Eval()->value);


Suggested change

c2.Eval()->value + 1 >= c1.Eval()->value);

c2.Eval()->value >= c1.Eval()->value);

?

The condition is correct here

wweic · 2019-03-12T20:12:46Z

src/arithmetic/rewrite_simplify.cc

+  TVM_TRY_REWRITE_IF(x < c1 && c2 < x, cfalse,
+                     c2.Eval()->value + 1 >= c1.Eval()->value);
+  TVM_TRY_REWRITE_IF(c2 < x && x < c1, cfalse,
+                     c2.Eval()->value + 1 >= c1.Eval()->value);


Suggested change

c2.Eval()->value + 1 >= c1.Eval()->value);

c2.Eval()->value >= c1.Eval()->value);

?

The condition is correct here

Yes you are right.

sgrechanik-h · 2019-03-13T13:05:56Z

src/arithmetic/rewrite_simplify.cc

@@ -1127,6 +1131,8 @@ Mutate_(const Not* op, const Expr& self) {
  TVM_TRY_REWRITE(!(x > y), x <= y);
  TVM_TRY_REWRITE(!(x == y), x != y);
  TVM_TRY_REWRITE(!(x != y), x == y);
+  TVM_TRY_REWRITE(!(x || y), (!x) && (!y));
+  TVM_TRY_REWRITE(!(x && y), (!x) || (!y));


It would be better to use TVM_TRY_RECURSIVE_REWRITE here, because otherwise nots are not propagated all the way down in some expressions (like ((max(max(i, x), y) <= 9) && ((x == i) || (y == i)))).

tqchen · 2019-03-14T05:13:24Z

Thanks, @sgrechanik-h @kazum @wweic please take another look. I tried the best to address some of the comments. I did not add all the rules suggested though, mainly because some of them could be added later, and we do not fully rely on the rewrite based simplification. The min/max chain reduction for example, can better be done through a canonical form that recognizes the min/max chain, which we can add later as another sub-analyzer.

tqchen · 2019-03-14T16:52:49Z

Thanks, @wweic @sgrechanik-h @kazum. This is now merged

[ARITH] RewriteSimplifier: min/max, logical, select

83d5a2a

tqchen added the status: need review label Mar 10, 2019

tqchen mentioned this pull request Mar 10, 2019

[ARITH] Analyzer RewriteSimplifier: add/sub/mul/div/mod #2722

Merged

3 tasks

sgrechanik-h reviewed Mar 11, 2019

View reviewed changes

src/arithmetic/rewrite_simplify.cc Show resolved Hide resolved

sgrechanik-h reviewed Mar 11, 2019

View reviewed changes

src/arithmetic/rewrite_simplify.cc Show resolved Hide resolved

update per review comment

b08e333

kazum requested changes Mar 12, 2019

View reviewed changes

wweic reviewed Mar 12, 2019

View reviewed changes

sgrechanik-h reviewed Mar 13, 2019

View reviewed changes

Update per comment

20c003f

wweic approved these changes Mar 14, 2019

View reviewed changes

sgrechanik-h approved these changes Mar 14, 2019

View reviewed changes

kazum approved these changes Mar 14, 2019

View reviewed changes

tqchen merged commit 046e4ff into apache:master Mar 14, 2019

tqchen deleted the rewrite2 branch March 15, 2019 01:39

wweic pushed a commit to wweic/tvm that referenced this pull request Mar 20, 2019

[ARITH] RewriteSimplifier: min/max, logical, select (apache#2768)

5a6d2a3

wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 20, 2019

[ARITH] RewriteSimplifier: min/max, logical, select (apache#2768)

e4ab5e9

tqchen mentioned this pull request Nov 8, 2019

[RELEASE][DRAFT] TVM v0.6 Release candidate #4259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ARITH] RewriteSimplifier: min/max, logical, select #2768

[ARITH] RewriteSimplifier: min/max, logical, select #2768

tqchen commented Mar 10, 2019

tqchen commented Mar 10, 2019

sgrechanik-h Mar 11, 2019

sgrechanik-h commented Mar 11, 2019 •

edited

Loading

tqchen commented Mar 12, 2019

kazum Mar 12, 2019

tqchen Mar 14, 2019

kazum Mar 12, 2019

kazum Mar 12, 2019

kazum Mar 12, 2019

wweic Mar 12, 2019

wweic Mar 12, 2019

wweic Mar 12, 2019

wweic Mar 12, 2019

tqchen Mar 14, 2019

wweic Mar 12, 2019

tqchen Mar 14, 2019

wweic Mar 14, 2019

sgrechanik-h Mar 13, 2019

tqchen Mar 14, 2019

tqchen commented Mar 14, 2019

tqchen commented Mar 14, 2019

	c2.Eval()->value + 1 >= c1.Eval()->value);
	c2.Eval()->value >= c1.Eval()->value);

[ARITH] RewriteSimplifier: min/max, logical, select #2768

[ARITH] RewriteSimplifier: min/max, logical, select #2768

Conversation

tqchen commented Mar 10, 2019

tqchen commented Mar 10, 2019

Choose a reason for hiding this comment

sgrechanik-h commented Mar 11, 2019 • edited Loading

tqchen commented Mar 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tqchen commented Mar 14, 2019

tqchen commented Mar 14, 2019

sgrechanik-h commented Mar 11, 2019 •

edited

Loading