mbs-octoml commented on a change in pull request #9669:
URL: https://github.com/apache/tvm/pull/9669#discussion_r766163313



##########
File path: src/relay/transforms/memory_alloc.cc
##########
@@ -91,9 +91,15 @@ class DialectRewriter : public 
transform::DeviceAwareExprMutator {
 
     for (auto field : tuple_node->fields) {
       auto new_field = Mutate(field);
-      if (new_field->IsInstance<ConstantNode>()) {
-        Var const_var("const", Type(nullptr));
-        new_field = scope.Push(const_var, new_field);
+      if (auto op = new_field.as<ConstantNode>()) {

Review comment:
       nit: const auto* op
   Seems to be the idiom (and matches my personal style :-)
   

##########
File path: src/relay/transforms/memory_alloc.cc
##########
@@ -91,9 +91,15 @@ class DialectRewriter : public 
transform::DeviceAwareExprMutator {
 
     for (auto field : tuple_node->fields) {
       auto new_field = Mutate(field);
-      if (new_field->IsInstance<ConstantNode>()) {
-        Var const_var("const", Type(nullptr));
-        new_field = scope.Push(const_var, new_field);
+      if (auto op = new_field.as<ConstantNode>()) {
+        DataType dtype = DataType(op->data->dtype);
+        bool is_simple_const = (dtype == DataType::Int(32) || dtype == 
DataType::Int(64) ||
+                                dtype == DataType::Float(32) || dtype == 
DataType::Float(64) ||
+                                dtype == DataType::Bool());
+        if (!op->is_scalar() || !is_simple_const) {
+          Var const_var("const", Type(nullptr));

Review comment:
       Thanks for this -- those two lines have been bugging me. Can you add a 
unit test to test_vm.py to demo the failure?

##########
File path: src/relay/transforms/memory_alloc.cc
##########
@@ -91,9 +91,15 @@ class DialectRewriter : public 
transform::DeviceAwareExprMutator {
 
     for (auto field : tuple_node->fields) {
       auto new_field = Mutate(field);
-      if (new_field->IsInstance<ConstantNode>()) {
-        Var const_var("const", Type(nullptr));
-        new_field = scope.Push(const_var, new_field);
+      if (auto op = new_field.as<ConstantNode>()) {
+        DataType dtype = DataType(op->data->dtype);

Review comment:
       nit: DataType dtype(op->data->dtype);
   

##########
File path: src/relay/op/vm/vm.cc
##########
@@ -41,6 +41,9 @@
 namespace tvm {
 namespace relay {
 
+// shape_of
+TVM_REGISTER_NODE_TYPE(ShapeOfAttrs);

Review comment:
       I've not seen this issue before, perhaps a linker bug. Could you leave a 
comment in case someone tries to move it back?




-- 
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]


Reply via email to