Support Object in visit API #54
Conversation
src/tvm/node/node.h
Outdated
| @@ -23,6 +23,12 @@ namespace runtime { | |||
| class NDArray; | |||
| } // namespace runtime | |||
|
|
|||
| namespace relay { | |||
| namespace vm { | |||
There was a problem hiding this comment.
perhaps make it runtime::vm::VMObject? The runtime namespace is isolated and can be compiled separately(in AOT)
There was a problem hiding this comment.
I was planning to rename right now, to simply the organization, just pushed a commit.
There was a problem hiding this comment.
is VMObject a reference class(like NDArray) or the container itself? Ideally we want it to be the reference class
There was a problem hiding this comment.
could likely be merged into the namspace above(runtime::NDArray)
There was a problem hiding this comment.
It is a reference class, we can move into runtime if you want
There was a problem hiding this comment.
You can make the decison. Perhaps it makes sense to move to runtime given that there is no other runtime Object
There was a problem hiding this comment.
yeah, not sure if we should keep it isolated or not. Maybe it makes sense if we will reuse for AoT/JiT, etc.
src/tvm/node/node.h
Outdated
| @@ -41,6 +46,7 @@ class EXPORT AttrVisitor { | |||
| virtual void Visit(const char* key, Type* value) = 0; | |||
| virtual void Visit(const char* key, NodeRef* value) = 0; | |||
| virtual void Visit(const char* key, runtime::NDArray* value) = 0; | |||
| virtual void Visit(const char* key, runtime::vm::Object* value) = 0; | |||
There was a problem hiding this comment.
Let us just do runtime::Object then
There was a problem hiding this comment.
I just decided right before you posted this ha. Just pushed a commit to do so.
This is part of the work in apache/tvm#2889, working on cleaning both up this will have to merge first in order to compile the other PR.