gemini-code-assist[bot] commented on code in PR #20029:
URL: https://github.com/apache/tvm/pull/20029#discussion_r3608976388


##########
src/tirx/ir/tir_visitor_with_path.cc:
##########
@@ -190,7 +190,7 @@ void TIRVisitorWithPath::VisitStmt_(const AttrStmtNode* op, 
AccessPath path) {
     // tirx::Var they annotate.
     context.push_back(WithDef(iter_var.value(), path->Attr("node")));

Review Comment:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   If `op->node` is `None`, `op->node.as<IterVar>()` will return an engaged 
`std::optional<IterVar>` containing an undefined `IterVar`. If `op->attr_key` 
matches `thread_extent` or `virtual_thread`, calling `WithDef` with this 
undefined `IterVar` will cause a crash or undefined behavior. We should guard 
the `WithDef` call by checking if `iter_var.value().defined()` is true.
   
   ```suggestion
       if (iter_var.value().defined()) {
         context.push_back(WithDef(iter_var.value(), path->Attr("node")));
       }
   ```



##########
src/tirx/ir/stmt.cc:
##########
@@ -92,8 +92,9 @@ TVM_FFI_STATIC_INIT_BLOCK() {
   refl::GlobalDef().def("tirx.AttrStmt",
                         [](Any node, ffi::String attr_key, PrimExpr value, 
Stmt body, Span span) {
                           // when node is a POD data type like int or bool, 
first convert to
-                          // primexpr.
-                          if (node.type_index() < 
ffi::TypeIndex::kTVMFFISmallStr) {
+                          // primexpr. Preserve None, which is a valid 
AttrStmt node value.
+                          if (node.type_index() > ffi::TypeIndex::kTVMFFINone 
&&
+                              node.type_index() < 
ffi::TypeIndex::kTVMFFISmallStr) {

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   Using `node.type_index() != ffi::TypeIndex::kTVMFFINone` is more explicit 
and robust than `>` to exclude the `None` type index, as it avoids any 
assumptions about the underlying integer values or signedness of the type index 
enum.
   
   ```c
                             if (node.type_index() != 
ffi::TypeIndex::kTVMFFINone &&
                                 node.type_index() < 
ffi::TypeIndex::kTVMFFISmallStr) {
   ```



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

Reply via email to