Node system lays the foundation to build TVM's AST structures. Each AST object 
corresponds to a subclass of Node. Besides the Node itself, we also usually 
defines a reference type which can be used to store a strong reference to the 
node. The following code example shows how to define Node ```XYZNode``` and 
reference type ```XYZ```.

```c++
class XYZ;

class XYZNode : public Node {
 public:
   static XYZ make(args);
};

class XYZ : public NodeRef {
  public:
   ...
};

void MyFunction() {
   XYZ ref = XYZNode::make(args);
}
```

Historically, we introduced a static factory function ```make``` to each Node 
type which creates a new reference to the Node(as shown in the above code). 
This is less natural compared to direct usage of a constructor or moves the 
static factory function into the reference class. As shown in below.

```c++
class XYZNode : public Node {
 public:
};

class XYZ : public NodeRef {
  public:
   XYZ(args);
   ...
};

void MyFunction() {
   // use constructor.
   XYZ ref = XYZ(args);
}
```

At the moment, not all the Node have a specific reference type -- this is also 
the reason why we adopt make in the Node historically. However, we start to see 
a trend of giving a specific reference type to each of the new Node 
introduced(e.g. in relay). I would also like to mention that we have already 
transitioned the python API to directly use the constructor-style code. Favor 
constructors in c++ will make both sides of the code to be more consistent.

This RFC tries to see if we want to migrate to the new style, specifically:

- For Nodes that do not have a corresponding reference type, keep the make 
function on Node.
- For Nodes that have a reference type
   - Favor constructor over XYZNode::make
   - Favor static factory functions in the reference type.

Most of the changes will be quite mechanical and we can do it incrementally. 
Please share your thoughts.





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3223

Reply via email to