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 2020/01/11 00:04:16 UTC
[GitHub] [incubator-tvm] yzhliu opened a new pull request #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
yzhliu opened a new pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684
To provide extra information for arith simplification.
This is an alternative approach for https://github.com/apache/incubator-tvm/pull/4486
More background,
https://discuss.tvm.ai/t/discuss-embed-more-bound-information-into-var-or-expr
https://discuss.tvm.ai/t/significant-increase-in-the-amount-of-cuda-code-gen-after-migrating-indexdiv-mod-to-floordiv-mod
@tqchen @icemelon9 Please review
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-574397147
https://discuss.tvm.ai/t/rfc-name-for-non-negative-variable
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
SizeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add SizeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-574898431
@tqchen I have address the comments and changed the name
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573985733
@tqchen updated per comments. do you think there could be a better name than `shape_var`? @icemelon9 reminded there is `ShapeVar` in relay, which represents an n-dim tuple.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu edited a comment on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu edited a comment on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573369731
@tqchen do you have any idea how come shape_var fails to be in func arg after I rebase from upstream today?
update,
nvm. find the issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-574285396
I don't have a better idea for name. Indeed shape could indicate a tuple rather than an integer.
We could potentially rename the relay's shape template variable for clarity as it is really type var. But we could think about a better name.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365612051
##########
File path: src/arithmetic/bound_deducer.cc
##########
@@ -297,6 +331,18 @@ void BoundDeducer::Transform() {
void BoundDeducer::Deduce() {
Init();
if (!success_) return;
+
+ // Any variable appears in both expr and result,
Review comment:
I do not understand the following comment. Why would n/4 be simplified to 0? if n>=0 the simplification rule does not stand, if n< 4, then it is a valid simplification
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4684: [Arith] add
SizeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4684: [Arith] add SizeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-574899548
cc @yzhliu feel free to merge after fixing the nits. BTW, it also reminds me if we would like to bring IterVar as a sub-class of Var, right now it takes Var as a member.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r366114637
##########
File path: src/arithmetic/bound_deducer.cc
##########
@@ -297,6 +331,18 @@ void BoundDeducer::Transform() {
void BoundDeducer::Deduce() {
Init();
if (!success_) return;
+
+ // Any variable appears in both expr and result,
Review comment:
as it is also using `IntervalSetEvaluator`, this is also not required after having shape_var return single_pt
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365636826
##########
File path: src/arithmetic/bound_deducer.cc
##########
@@ -297,6 +331,18 @@ void BoundDeducer::Transform() {
void BoundDeducer::Deduce() {
Init();
if (!success_) return;
+
+ // Any variable appears in both expr and result,
Review comment:
for `>=`, current logic is to first get the lower bound of the left side then deduce for `i`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365970269
##########
File path: src/arithmetic/int_set.cc
##########
@@ -529,6 +554,22 @@ class IntervalSetEvaluator :
return Combine<T>(analyzer_, a, b);
}
+ template<typename T>
+ inline IntervalSet VisitDivExpr_(const T* op) {
+ IntervalSet a = this->Eval(op->a);
+ IntervalSet b = this->Eval(op->b);
+ if ((MatchPoint(a, op->a) && (MatchPoint(b, op->b) || SelfBoundedVar(b, op->b)))
+ || (SelfBoundedVar(a, op->a) && SelfBoundedVar(b, op->b))) {
+ // e.g.,
Review comment:
yeah... you're right.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365928309
##########
File path: src/arithmetic/int_set.cc
##########
@@ -529,6 +554,22 @@ class IntervalSetEvaluator :
return Combine<T>(analyzer_, a, b);
}
+ template<typename T>
+ inline IntervalSet VisitDivExpr_(const T* op) {
+ IntervalSet a = this->Eval(op->a);
+ IntervalSet b = this->Eval(op->b);
+ if ((MatchPoint(a, op->a) && (MatchPoint(b, op->b) || SelfBoundedVar(b, op->b)))
+ || (SelfBoundedVar(a, op->a) && SelfBoundedVar(b, op->b))) {
+ // e.g.,
Review comment:
i think this can be fixed by having shape_var return single_pt
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365612326
##########
File path: src/arithmetic/int_set.cc
##########
@@ -529,6 +554,22 @@ class IntervalSetEvaluator :
return Combine<T>(analyzer_, a, b);
}
+ template<typename T>
+ inline IntervalSet VisitDivExpr_(const T* op) {
+ IntervalSet a = this->Eval(op->a);
+ IntervalSet b = this->Eval(op->b);
+ if ((MatchPoint(a, op->a) && (MatchPoint(b, op->b) || SelfBoundedVar(b, op->b)))
+ || (SelfBoundedVar(a, op->a) && SelfBoundedVar(b, op->b))) {
+ // e.g.,
Review comment:
This part is a bit confusing. If the divisor is already a set(which means it is relaxed), we should not return the original var.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573453462
passing var into the compute still works
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365638275
##########
File path: src/arithmetic/int_set.cc
##########
@@ -529,6 +554,22 @@ class IntervalSetEvaluator :
return Combine<T>(analyzer_, a, b);
}
+ template<typename T>
+ inline IntervalSet VisitDivExpr_(const T* op) {
+ IntervalSet a = this->Eval(op->a);
+ IntervalSet b = this->Eval(op->b);
+ if ((MatchPoint(a, op->a) && (MatchPoint(b, op->b) || SelfBoundedVar(b, op->b)))
+ || (SelfBoundedVar(a, op->a) && SelfBoundedVar(b, op->b))) {
+ // e.g.,
Review comment:
but in `Combine<Div>` such case evaluates to `IntervalSet::Everything()`.
e.g., `4 / tvm.var() => 4 / tvm.var()`, while `4 / tvm.shape_var() => (-inf, +inf)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-574380538
We can ask other's thoughts about it(perhaps list a few candidates and send an rfc?). TIndex seems to be a reasonable name, although it is a bit ambiguous because people need to guess what does T mean).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573452486
cc @ZihengJiang @Hzfengsy would be great if you can help to take a look
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365970115
##########
File path: src/arithmetic/bound_deducer.cc
##########
@@ -297,6 +331,18 @@ void BoundDeducer::Transform() {
void BoundDeducer::Deduce() {
Init();
if (!success_) return;
+
+ // Any variable appears in both expr and result,
Review comment:
I believe it is just a more strict bound.
I agree, let me have a try to see if it is feasible.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r366473075
##########
File path: include/tvm/expr.h
##########
@@ -115,6 +117,45 @@ class Var : public PrimExpr {
using ContainerType = VarNode;
};
+class ShapeVar;
+/*!
+ * \brief A variable node represent a tensor shape size,
+ * whose value must be non-negative.
+ */
+class ShapeVarNode : public VarNode {
+ public:
+ /*! \brief constructor */
+ ShapeVarNode() {}
+ ShapeVarNode(DataType dtype, std::string name_hint);
+
+ static constexpr const char* _type_key = "ShapeVar";
+ TVM_DECLARE_FINAL_OBJECT_INFO(ShapeVarNode, VarNode);
+};
+
+/*! \brief a named variable represents a tensor shape size */
+class ShapeVar : public Var {
+ public:
+ explicit ShapeVar(ObjectPtr<Object> n) : Var(n) {}
+ TVM_DLL explicit ShapeVar(std::string name_hint = "s",
Review comment:
Document the constructor
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365611921
##########
File path: src/arithmetic/bound_deducer.cc
##########
@@ -70,6 +70,40 @@ std::vector<const Object*> GetPath(PrimExpr target, PrimExpr expr) {
return v.path_;
}
+class BoundRemover : public ExprMutator {
+ public:
+ PrimExpr Remove(const PrimExpr& e) {
+ remove_bounded_ = true;
Review comment:
Can you document its behavior? Is it still necessary, given bound info is no longer wrapped in the assert_
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu merged pull request #4684: [Arith] add
SizeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu merged pull request #4684: [Arith] add SizeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365612232
##########
File path: src/arithmetic/int_set.cc
##########
@@ -519,6 +536,14 @@ class IntervalSetEvaluator :
return set->min_value.same_as(value) && set->max_value.same_as(value);
}
+ bool SelfBoundedVar(const IntervalSet& set,
+ const PrimExpr& value) const {
Review comment:
need comment about the function name.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu edited a comment on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu edited a comment on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573369731
@tqchen do you have any idea how come shape_var fails to be in func arg after I rebase from upstream today?
---
nvm. find the issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365638409
##########
File path: src/arithmetic/int_set.cc
##########
@@ -405,6 +405,23 @@ class IntervalSetEvaluator :
}
}
+ IntervalSet VisitExpr_(const ShapeVarNode* op) final {
+ Var var = GetRef<Var>(op);
+ auto it = dom_map_.find(var);
+ if (it != dom_map_.end()) {
+ IntervalSet res = ToIntervalSet((*it).second);
+ if (res->min_value.same_as(var) &&
+ res->max_value.same_as(var)) {
+ return res;
+ }
+ // recursively evaluate mapped result
+ // in case the domain contains variables to be relaxed.
+ return Eval(res);
+ } else {
+ return IntervalSet(0, GetRef<ShapeVar>(op));
Review comment:
fair. I'll change and verify.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365928309
##########
File path: src/arithmetic/int_set.cc
##########
@@ -529,6 +554,22 @@ class IntervalSetEvaluator :
return Combine<T>(analyzer_, a, b);
}
+ template<typename T>
+ inline IntervalSet VisitDivExpr_(const T* op) {
+ IntervalSet a = this->Eval(op->a);
+ IntervalSet b = this->Eval(op->b);
+ if ((MatchPoint(a, op->a) && (MatchPoint(b, op->b) || SelfBoundedVar(b, op->b)))
+ || (SelfBoundedVar(a, op->a) && SelfBoundedVar(b, op->b))) {
+ // e.g.,
Review comment:
i think this can be fixed by having shape_var return single_pt(as in the other comment)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365607382
##########
File path: include/tvm/expr.h
##########
@@ -115,6 +115,43 @@ class Var : public PrimExpr {
using ContainerType = VarNode;
};
+class ShapeVar;
+/*!
+ * \brief A variable node represent a tensor shape size,
+ * whose value must be non-negative.
+ */
+class ShapeVarNode : public VarNode {
+ public:
+ static ShapeVar make(DataType dtype, std::string name_hint);
Review comment:
use constructor in ShapeVar, as per https://github.com/apache/incubator-tvm/issues/4648
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365607448
##########
File path: python/tvm/build_module.py
##########
@@ -309,7 +313,7 @@ def get_binds(args, compact=False, binds=None):
arg_list.append(binds[x])
elif isinstance(x, schedule.Buffer):
arg_list.append(x)
- elif isinstance(x, expr.Var):
Review comment:
keep the original one, as we can make ShapeVar as subclass expr.Var
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu edited a comment on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu edited a comment on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573369731
@tqchen do you have any idea how come shape_var fails to be in func arg after I rebase from upstream today?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu edited a comment on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu edited a comment on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573985733
@tqchen updated per comments. do you think there could be a better name than `shape_var`? @icemelon9 reminded there is `ShapeVar` in relay, which represents an n-dim tuple I believe https://github.com/apache/incubator-tvm/blob/master/python/tvm/relay/ty.py#L145
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573384531
@tqchen @icemelon9 CI's green. it's ready for review.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
SizeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add SizeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-574999427
Thanks @tqchen I'll take a look into the IndexVar and send another pr
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add SizeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add SizeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r367154337
##########
File path: python/tvm/api.py
##########
@@ -192,6 +192,25 @@ def var(name="tindex", dtype=int32):
return _api_internal._Var(name, dtype)
+def size_var(name="tindex", dtype=int32):
Review comment:
is tindex a good default? size?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365612202
##########
File path: src/arithmetic/int_set.cc
##########
@@ -405,6 +405,23 @@ class IntervalSetEvaluator :
}
}
+ IntervalSet VisitExpr_(const ShapeVarNode* op) final {
+ Var var = GetRef<Var>(op);
+ auto it = dom_map_.find(var);
+ if (it != dom_map_.end()) {
+ IntervalSet res = ToIntervalSet((*it).second);
+ if (res->min_value.same_as(var) &&
+ res->max_value.same_as(var)) {
+ return res;
+ }
+ // recursively evaluate mapped result
+ // in case the domain contains variables to be relaxed.
+ return Eval(res);
+ } else {
+ return IntervalSet(0, GetRef<ShapeVar>(op));
Review comment:
Shouldn't it be IntervalSet::SinglePoint(var) as in Var? This is a bit overly relaxed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365638275
##########
File path: src/arithmetic/int_set.cc
##########
@@ -529,6 +554,22 @@ class IntervalSetEvaluator :
return Combine<T>(analyzer_, a, b);
}
+ template<typename T>
+ inline IntervalSet VisitDivExpr_(const T* op) {
+ IntervalSet a = this->Eval(op->a);
+ IntervalSet b = this->Eval(op->b);
+ if ((MatchPoint(a, op->a) && (MatchPoint(b, op->b) || SelfBoundedVar(b, op->b)))
+ || (SelfBoundedVar(a, op->a) && SelfBoundedVar(b, op->b))) {
+ // e.g.,
Review comment:
but in `Everything<Div>` such case evaluates to `IntervalSet::Everything()`.
e.g., `4 / tvm.var() => 4 / tvm.var()`, while `4 / tvm.shape_var() => (-inf, +inf)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573369731
@tqchen do you have any idea how shape_var fails to be in func arg after I rebase from upstream today?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4684:
[Arith] add ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#discussion_r365928076
##########
File path: src/arithmetic/bound_deducer.cc
##########
@@ -297,6 +331,18 @@ void BoundDeducer::Transform() {
void BoundDeducer::Deduce() {
Init();
if (!success_) return;
+
+ // Any variable appears in both expr and result,
Review comment:
I see, perhaps we can add more explanation then.
Also, would it results in an incorrect bound? or just a more strict bound(which still makes the condition holds)
For a better approach. My feeling is that we should first have a rewriting that tries to move vars into one side
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu commented on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu commented on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-574323153
How about `TIndexVar` stands for tensor index variable?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] yzhliu edited a comment on issue #4684: [Arith] add
ShapeVar representing non-neg valued variable in a tensor shape
Posted by GitBox <gi...@apache.org>.
yzhliu edited a comment on issue #4684: [Arith] add ShapeVar representing non-neg valued variable in a tensor shape
URL: https://github.com/apache/incubator-tvm/pull/4684#issuecomment-573369731
@tqchen do you have any idea how come shape_var fails to be in func arg after I rebase from upstream today?
-
nvm. find the issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services