slyubomirsky commented on code in PR #16356:
URL: https://github.com/apache/tvm/pull/16356#discussion_r1480525572


##########
src/script/printer/relax/utils.h:
##########
@@ -82,10 +84,47 @@ inline Optional<ExprDoc> StructInfoAsAnn(const relax::Var& 
v, const ObjectPath&
   if (!v->struct_info_.defined()) {
     return NullOpt;
   }
+  bool attempt_to_hide_struct_info = !d->cfg->show_all_struct_info;
+
   if (const auto* call = rhs.as<relax::CallNode>()) {
     static const Op& call_tir_op = Op::Get("relax.call_tir");
     static const Op& call_dps_packed_op = Op::Get("relax.call_dps_packed");
     if (call->op.same_as(call_tir_op) || call->op.same_as(call_dps_packed_op)) 
{
+      attempt_to_hide_struct_info = true;
+    }
+  }
+  if (attempt_to_hide_struct_info) {
+    Optional<relax::StructInfo> inferred_sinfo = NullOpt;

Review Comment:
   Could you explain why you included all this logic? I would not assume that 
the printer would have to _run_ any type inference of its own. If the struct 
info hasn't been filled in by the normalizer, I'm not sure it's the job of the 
printer to do that.



##########
src/script/printer/relax/utils.h:
##########
@@ -82,10 +84,47 @@ inline Optional<ExprDoc> StructInfoAsAnn(const relax::Var& 
v, const ObjectPath&
   if (!v->struct_info_.defined()) {
     return NullOpt;
   }
+  bool attempt_to_hide_struct_info = !d->cfg->show_all_struct_info;
+
   if (const auto* call = rhs.as<relax::CallNode>()) {
     static const Op& call_tir_op = Op::Get("relax.call_tir");
     static const Op& call_dps_packed_op = Op::Get("relax.call_dps_packed");
     if (call->op.same_as(call_tir_op) || call->op.same_as(call_dps_packed_op)) 
{
+      attempt_to_hide_struct_info = true;
+    }
+  }
+  if (attempt_to_hide_struct_info) {
+    Optional<relax::StructInfo> inferred_sinfo = NullOpt;

Review Comment:
   Could you explain why you included all this logic? I would not assume that 
the printer would have to _run_ any type inference of its own. If the struct 
info hasn't been filled in by the normalizer, I'm not sure it's the job of the 
printer to do that.



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