You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/04/26 14:49:57 UTC

[GitHub] [tvm] tqchen commented on a diff in pull request #11130: [Analysis] Readability/Deduplication in Analyzer CanProve/Simplify

tqchen commented on code in PR #11130:
URL: https://github.com/apache/tvm/pull/11130#discussion_r858811057


##########
src/arith/analyzer.cc:
##########
@@ -108,29 +108,25 @@ bool Analyzer::CanProveEqual(const PrimExpr& lhs, const PrimExpr& rhs) {
 }
 
 bool Analyzer::CanProve(const PrimExpr& expr) {
-  if (const auto* ptr = expr.as<IntImmNode>()) {
-    return ptr->value != 0;
-  }
-  auto res = this->rewrite_simplify(expr);
-  if (const auto* ptr = res.as<IntImmNode>()) {
-    return ptr->value != 0;
-  }
-  res = this->canonical_simplify(expr);
-  if (const auto* ptr = res.as<IntImmNode>()) {
-    return ptr->value != 0;
-  }
-  return false;
+  PrimExpr simplified = Simplify(expr);
+  const int64_t* as_int = tir::as_const_int(simplified);
+  return as_int && *as_int;
 }
 
 PrimExpr Analyzer::Simplify(const PrimExpr& expr, int steps) {
-  if (tir::is_const_int(expr)) return expr;
   PrimExpr res = expr;
-  for (int i = 0; i < steps; ++i) {
-    res = this->rewrite_simplify(res);
-    if (tir::is_const_int(res) || ++i == steps) return res;
-    res = this->canonical_simplify(res);
-    if (tir::is_const_int(res)) return res;
+
+  for (int i = 0; i < steps; i++) {
+    if (tir::is_const_int(res)) {

Review Comment:
   we can do return res here.



##########
src/arith/analyzer.cc:
##########
@@ -108,29 +108,25 @@ bool Analyzer::CanProveEqual(const PrimExpr& lhs, const PrimExpr& rhs) {
 }
 
 bool Analyzer::CanProve(const PrimExpr& expr) {
-  if (const auto* ptr = expr.as<IntImmNode>()) {
-    return ptr->value != 0;

Review Comment:
   These are quick path that we intended leave as it is to avoid trigerring of recursive function calling cost. Given CanProve can be called frequently, it would be useful to keep it as it is in this case.
   
   We can document the rationle using comments



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org