gemini-code-assist[bot] commented on code in PR #19583:
URL: https://github.com/apache/tvm/pull/19583#discussion_r3257666322
##########
src/script/printer/ir/ir.cc:
##########
@@ -35,15 +35,24 @@ struct SortableFunction {
: priority(0), gv(obj.first), func(obj.second) {
if (gv->name_hint == "main") {
priority = 1000;
- } else if (obj.second->GetTypeKey() == "tirx.PrimFunc") {
- priority = 1;
- } else if (obj.second->GetTypeKey() == "relax.expr.ExternFunc") {
- priority = 2;
- } else if (obj.second->GetTypeKey() == "relax.expr.Function") {
- priority = 3;
+ } else if (func.defined()) {
+ if (func->GetTypeKey() == "tirx.PrimFunc") {
+ priority = 1;
+ } else if (func->GetTypeKey() == "relax.expr.ExternFunc") {
+ priority = 2;
+ } else if (func->GetTypeKey() == "relax.expr.Function") {
+ priority = 3;
+ } else {
+ TVM_FFI_THROW(TypeError) << "TVMScript cannot print functions of type:
"
+ << func->GetTypeKey();
+ }
} else {
- TVM_FFI_THROW(TypeError) << "TVMScript cannot print functions of type: "
- << obj.second->GetTypeKey();
+ // PrimFuncPass may leave undefined GlobalVar slots when transforming
+ // this function (see tirx/ir/transform.cc); this transient state may
+ // be encountered during the internal call Dump(mod) executed in
+ // PrimFuncPass during debugging.
+ priority = 999;
+ LOG(INFO) << "Function " << gv->name_hint << " is undefined";
}
Review Comment:
The current implementation has two issues:
1. It skips logging for the `main` function if it is undefined, which is
inconsistent with how other functions are handled. If `main` is the function
being transformed, it will be undefined and should be logged as such.
2. While this change prevents a crash during the sorting phase, the printing
loop (starting at line 97) will still attempt to use the undefined `base_func`.
Specifically, line 117 will crash when calling `base_func->GetTypeKey()` if the
function is undefined. You should also guard the printing loop to skip
undefined functions or emit a placeholder Doc.
I suggest refactoring the constructor to handle the undefined case first and
consistently for all functions, including `main`.
```c
if (!func.defined()) {
// PrimFuncPass may leave undefined GlobalVar slots when transforming
// this function (see tirx/ir/transform.cc); this transient state may
// be encountered during the internal call Dump(mod) executed in
// PrimFuncPass during debugging.
priority = (gv->name_hint == "main") ? 1000 : 999;
LOG(INFO) << "Function " << gv->name_hint << " is undefined";
} else if (gv->name_hint == "main") {
priority = 1000;
} else if (func->GetTypeKey() == "tirx.PrimFunc") {
priority = 1;
} else if (func->GetTypeKey() == "relax.expr.ExternFunc") {
priority = 2;
} else if (func->GetTypeKey() == "relax.expr.Function") {
priority = 3;
} else {
TVM_FFI_THROW(TypeError) << "TVMScript cannot print functions of type:
"
<< func->GetTypeKey();
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
