comaniac commented on a change in pull request #9641:
URL: https://github.com/apache/tvm/pull/9641#discussion_r764397512
##########
File path: include/tvm/relay/expr.h
##########
@@ -240,14 +245,16 @@ class Var : public Expr {
* \param opt_vid The (optional) vid for the copied var. If none, ret_var->vid
= var->vid.
* \param opt_type_annotation The (optional) type_annotation for the copied
var. If none,
* ret_var->type_annotation = var->type_annotation.
- * \param opt_span The (optional) span for the copied var. If none,
ret_var->span = var->span.
- * \return If all properties are null or the same as the property in the input
var
- * (i.e., opt_vid is null or opt_vid.value() == var->vid, etc.), then we
return var. Otherwise,
- * we return a copy of call with the different fields overwritten. (i.e., if
- * opt_vid.value() != var->vid, then ret_var->vid = opt_.value()).
+ * \param opt_virtual_device The (optional) virtual_device for the copied
tuple. If none,
+ * ret_tuple->virtual_device = tuple->virtual_device. \param opt_span The
(optional) span for the
+ * copied var. If none, ret_var->span = var->span. \return If all properties
are null or the same as
+ * the property in the input var (i.e., opt_vid is null or opt_vid.value() ==
var->vid, etc.), then
+ * we return var. Otherwise, we return a copy of call with the different
fields overwritten. (i.e.,
+ * if opt_vid.value() != var->vid, then ret_var->vid = opt_.value()).
Review comment:
\param and \return need new lines.
##########
File path: include/tvm/relay/expr.h
##########
@@ -456,15 +465,17 @@ class Let : public Expr {
* \param opt_var The (optional) var for the copied let. If none, ret_let->op
= let->op.
* \param opt_value The (optional) value for the copied let. If none,
ret_let->args = let->args.
* \param opt_body The (optional) body for the copied let. If none,
ret_let->attrs = let->attrs.
- * \param opt_span The (optional) span for the copied let. If none,
ret_let->span = let->span.
- * \return If all properties are null or the same as the property in the input
let (i.e., opt_var is
- * null or opt_var.value() == let->var, etc.), then we return let. Otherwise,
we return a copy of
- * let with the different fields overwritten. (i.e., if opt_var.value() !=
let->var, then
- * ret_let->var = opt_var.value()).
+ * \param opt_virtual_device The (optional) virtual_device for the copied let.
If none,
+ * ret_let->virtual_device = let->virtual_device. \param opt_span The
(optional) span for the copied
+ * let. If none, ret_let->span = let->span. \return If all properties are null
or the same as the
+ * property in the input let (i.e., opt_var is null or opt_var.value() ==
let->var, etc.), then we
+ * return let. Otherwise, we return a copy of let with the different fields
overwritten. (i.e., if
+ * opt_var.value() != let->var, then ret_let->var = opt_var.value()).
Review comment:
ditto
##########
File path: include/tvm/relay/expr.h
##########
@@ -784,16 +804,18 @@ class RefWrite : public Expr {
* ret_ref_write->ref = ref_write->ref.
* \param opt_value The (optional) value for the copied ref_write. If none,
* ret_ref_write->value = ref_write->value.
- * \param opt_span
- * The (optional) span for the copied ref_write. If none, ret_ref_write->span
= ref_write->span.
- * \return If all properties are null or the same as the property in the input
ref_write
- * (i.e., opt_ref is null or opt_ref.value() == ref_write->ref, etc.), then we
return ref_write.
- * Otherwise, we return a copy of ref_write with the different fields
overwritten.
- * (i.e., if ref_write.value() != ref_write->ref, then
- * ret_ref_write->ref = opt_ref.value()).
+ * \param opt_virtual_device
+ * The (optional) virtual_device for the copied ref_write. If none,
ret_ref_write->virtual_device =
+ * ref_write->virtual_device. \param opt_span The (optional) span for the
copied ref_write. If none,
+ * ret_ref_write->span = ref_write->span. \return If all properties are null
or the same as the
+ * property in the input ref_write (i.e., opt_ref is null or opt_ref.value()
== ref_write->ref,
+ * etc.), then we return ref_write. Otherwise, we return a copy of ref_write
with the different
+ * fields overwritten. (i.e., if ref_write.value() != ref_write->ref, then
ret_ref_write->ref =
+ * opt_ref.value()).
Review comment:
ditto
##########
File path: include/tvm/ir/expr.h
##########
@@ -165,6 +168,30 @@ class RelayExprNode : public BaseExprNode {
template <typename TTypeNode>
inline const TTypeNode* type_as() const;
+ /*!
+ * \brief The virtual device (SEScope) for this node (the result of device
planning).
+ * For first-order expressions (non functions), this describes where the
result of evaluating the
+ * expression should be stored. Note that currently, all composite
first-order values (tuples,
+ * references, ADTs) must be stored on the same virtual device. This means
that it is not possible
+ * to store two tuple fields on different devices, so we only need one
virtual device for these
+ * types.
+ *
+ * For expressions that have the function type, the virtual device describes
where the result of
+ * the call to the function or closure is stored (instead of where the
function itself is stored).
+ * The SEScope's Target field describes how the body of the function should
be compiled.
+ *
+ * \note Unfortunately, the type of virtual_device_ needs to be ObjectRef to
avoid a circular
+ * import. We can forward-declare the SEScope type for the getter function,
but not for the field
+ * itself.
Review comment:
nit: feel that this would be sufficient to explain the reason of using
ObjectRef here.
```suggestion
* \note The type of virtual_device_ needs to be ObjectRef to avoid a
circular import.
```
##########
File path: include/tvm/relay/expr.h
##########
@@ -720,15 +738,17 @@ class RefRead : public Expr {
* \param ref_read The ref_read to copy.
* \param opt_ref The (optional) ref for the copied ref_read. If none,
ret_ref_read->ref =
* ref_read->ref.
- * \param opt_span
- * The (optional) span for the copied ref_read. If none, ret_ref_read->span =
ref_read->span.
- * \return If all properties are null or the same as the property in the input
ref_read
- * (i.e., opt_ref is null or opt_ref.value() == ref_read->ref, etc.), then we
return ref_read.
- * Otherwise, we return a copy of ref_read with the different fields
overwritten.
- * (i.e., if opt_ref.value() != ref_read->ref, then
- * ret_ref_read->ref = opt_ref.value()).
+ * \param opt_virtual_device
+ * The (optional) virtual_device for the copied ref_read. If none,
ret_ref_read->virtual_device =
+ * ref_read->virtual_device. \param opt_span The (optional) span for the
copied ref_read. If none,
+ * ret_ref_read->span = ref_read->span. \return If all properties are null or
the same as the
+ * property in the input ref_read (i.e., opt_ref is null or opt_ref.value() ==
ref_read->ref, etc.),
+ * then we return ref_read. Otherwise, we return a copy of ref_read with the
different fields
+ * overwritten. (i.e., if opt_ref.value() != ref_read->ref, then
ret_ref_read->ref =
+ * opt_ref.value()).
Review comment:
ditto
##########
File path: include/tvm/relay/expr.h
##########
@@ -362,16 +369,18 @@ class Call : public Expr {
* call->attrs.
* \param opt_type_args The (optional) type args for the copied call. If none,
* ret_call->type_args = call->type_args.
- * \param opt_span The (optional) span for the copied call. If none,
ret_call->span = call->span.
- * \return If all properties are null or the same as the property in the input
call
- * (i.e., opt_op is null or opt_op.value() == call->op, etc.), then we return
call. Otherwise, we
- * return a copy of call with the different fields overwritten. (i.e., if
opt_op.value() !=
- * call->op, then ret_call->op = opt_op.value()).
+ * \param opt_virtual_device The (optional) virtual_device for the copied
call. If none,
+ * ret_call->virtual_device = call->virtual_device. \param opt_span The
(optional) span for the
+ * copied call. If none, ret_call->span = call->span. \return If all
properties are null or the same
+ * as the property in the input call (i.e., opt_op is null or opt_op.value()
== call->op, etc.),
+ * then we return call. Otherwise, we return a copy of call with the different
fields overwritten.
+ * (i.e., if opt_op.value() != call->op, then ret_call->op = opt_op.value()).
Review comment:
ditto
##########
File path: include/tvm/relay/expr.h
##########
@@ -539,17 +550,19 @@ class If : public Expr {
* ret_if->true_branch = ret_if->false_branch.
* \param opt_false_branch The (optional) false_branch
* for the copied if_expr. If none, ret_if->false_branch =
if_expr->false_branch.
- * \param opt_span
- * The (optional) span for the copied if_expr. If none, ret_if->span =
if_expr->span.
- * \return If all
- * properties are null or the same as the property in the input if_expr (i.e.,
opt_cond is null or
- * opt_cond.value() == if_expr->cond, etc.), then we return if_expr.
Otherwise, we return a copy of
- * if_expr with the different fields overwritten. (i.e., if opt_cond.value()
!= if_expr->cond, then
- * ret_if->cond = opt_cond.value()).
+ * \param opt_virtual_device The (optional) virtual_device for the copied
if_expr. If none,
+ * ret_if->virtual_device = if_expr->virtual_device.
+ * \param opt_span The (optional) span for the copied if_expr. If none,
+ * ret_if->span = if_expr->span.
+ * \return If all properties are null or the same as the property in
+ * the input if_expr (i.e., opt_cond is null or opt_cond.value() ==
if_expr->cond, etc.), then we
+ * return if_expr. Otherwise, we return a copy of if_expr with the different
fields overwritten.
+ * (i.e., if opt_cond.value() != if_expr->cond, then ret_if->cond =
opt_cond.value()).
Review comment:
ditto
--
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]
