Lunderberg commented on code in PR #13933:
URL: https://github.com/apache/tvm/pull/13933#discussion_r1102921815


##########
src/arith/rewrite_simplify.cc:
##########
@@ -228,10 +228,13 @@ PrimExpr RewriteSimplifier::Impl::VisitExpr_(const 
AddNode* op) {
     TVM_TRY_REWRITE_IF(max(y + z * c1, x) + z * c2, max(x + z * c2, y),
                        c1.Eval()->value == -c2.Eval()->value);
 
-    TVM_TRY_REWRITE(max(x, y) + min(x, y), x + y);
-    TVM_TRY_REWRITE(min(x, y) + max(x, y), x + y);
-    TVM_TRY_REWRITE(max(x, y) + min(y, x), x + y);
-    TVM_TRY_REWRITE(min(x, y) + max(y, x), x + y);
+    TVM_TRY_REWRITE((PMatchesOneOf{

Review Comment:
   Good point.  This same pitfall exists in the existing `TVM_TRY_REWRITE`, 
that a replacement rule could make use of a pattern that isn't defined by the 
match itself, but it would be easier to make the mistake here.  I've added a 
note next to the macro definitions that the `ResExpr` should only be in terms 
of `PVar`s that would be defined within every possible match.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to