Hzfengsy commented on code in PR #12450:
URL: https://github.com/apache/tvm/pull/12450#discussion_r953882199


##########
src/tir/schedule/primitive/compute_at.cc:
##########
@@ -160,7 +174,57 @@ int FindInsertionPoint(
   // The valid indices are: (last_producer_position, first_consumer_position]
   ICHECK(split.last_producer_position < split.first_consumer_position);
   // Step 4. Return the last valid insertion point
-  return split.first_consumer_position;
+  int insert_position = split.first_consumer_position;
+  if (index == -1) {
+    return insert_position;
+  } else if (index == -2) {
+    if (require_all_producers_visited) {
+      if (split.last_producer_position >= 0) {
+        insert_position = split.last_producer_position + 1;
+      } else {
+        insert_position = split.first_consumer_position;
+      }

Review Comment:
   This line does not make sense to me. `split.last_producer_position` should 
always be `>=-1`, so `split.last_producer_position + 1`, which is `>=0` is 
always a valid position for `index == -2`
   
   lmk if I make some mistakes.



##########
src/tir/schedule/primitive/compute_at.cc:
##########
@@ -160,7 +174,57 @@ int FindInsertionPoint(
   // The valid indices are: (last_producer_position, first_consumer_position]
   ICHECK(split.last_producer_position < split.first_consumer_position);
   // Step 4. Return the last valid insertion point
-  return split.first_consumer_position;
+  int insert_position = split.first_consumer_position;
+  if (index == -1) {
+    return insert_position;
+  } else if (index == -2) {

Review Comment:
   The logic for `index == -2` is so confused. Based on the branch `index > 0`, 
all possible location are always `(last_producer_position, 
first_consumer_position]`, so can we just return `last_producer_position + 1` 
in this case?



##########
src/tir/schedule/primitive/compute_at.cc:
##########
@@ -160,7 +174,57 @@ int FindInsertionPoint(
   // The valid indices are: (last_producer_position, first_consumer_position]
   ICHECK(split.last_producer_position < split.first_consumer_position);
   // Step 4. Return the last valid insertion point

Review Comment:
   Please update the comment here.



-- 
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: commits-unsubscr...@tvm.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to