Lunderberg commented on code in PR #16859:
URL: https://github.com/apache/tvm/pull/16859#discussion_r1572728664
##########
include/tvm/relax/expr.h:
##########
@@ -915,18 +843,113 @@ class SeqExprNode : public ExprNode {
class SeqExpr : public Expr {
public:
+ /* \brief Implicit conversion constructor
+ *
+ * Relax nodes that introduce a new scope (e.g. `relax::Function`)
+ * are required to be held as SeqExpr. This implicit conversion
+ * provides allows callsites to use these member variables when the
+ * C++ compile-time type is a `relax::Expr`. For example,
+ * a transform may use `func.CopyOnWrite()->body = expr;`.
+ *
+ * If the expression is already a `relax::SeqExpr`, the same
+ * underlying `relax::SeqExprNode` is used, and no copies are made.
+ */
+ TVM_DLL SeqExpr(Expr body); // NOLINT(*)
+
TVM_DLL explicit SeqExpr(Array<BindingBlock> blocks, Expr body, Span span =
Span());
TVM_DEFINE_OBJECT_REF_METHODS(SeqExpr, Expr, SeqExprNode);
TVM_DEFINE_OBJECT_REF_COW_METHOD(SeqExprNode);
};
+/*!
+ * \brief Condition expression
+ *
+ * Unlike traditional statement `if`s, the if evalutes
+ * to the result of the branch taken.
+ *
+ * x = if (true) { 1 } else { 0 }; // x is 1
+ * y = if (false) { 1 } else { 0 }; // y is 0
+ *
+ * \note This is similar to C's ternary operator.
+ */
+class IfNode : public ExprNode {
Review Comment:
Unfortunately, I wasn't able to do so. Declaring an member variable in a
class requires the size of that member variable to be defined when the member
variable is declared. For cases where the size of a member variable is
independent of the size of `T` (e.g. `T*`, `const T&`, or some templates such
as `std::unique_ptr<T>`), a forward-declaration can be used. Since the
`relax::SeqExpr` is held by value, it cannot be defined with a
forward-reference, and so the definitions of `relax::Function` and `relax::If`
must occur after `relax::SeqExpr`.
(Even though all `ObjectRef` subclasses store their data in a
reference-managed pointer, and have the exact same size, I'm not aware of any
way to expose this to the compiler in a forward declaration. A few years ago,
I looked into what it would take to turn `ObjectRef` into a template over the
internal object (e.g. `using SeqExpr = ObjectRefImpl<SeqExprNode>;`), and
decided that it would be a bit more effort than I'd like. It may be worth
doing so, with care to ensure that a forward reference of `SeqExprNode` would
be sufficient to define `ObjectRefImpl<SeqExprNode>`, but that would be a much
larger change.)
--
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-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
