You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/09/15 19:10:49 UTC

[GitHub] [tvm] vinx13 opened a new pull request, #12797: [TIR] Construct the inverse in SuggestIndexMap

vinx13 opened a new pull request, #12797:
URL: https://github.com/apache/tvm/pull/12797

   Computing the inverse mapping requires arithmetic analysis which is not guaranteed to cover all cases. We provide the pre-defined inverse index map instead.
   cc @junrushao 


-- 
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-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] junrushao merged pull request #12797: [TIR] Construct the inverse in SuggestIndexMap

Posted by GitBox <gi...@apache.org>.
junrushao merged PR #12797:
URL: https://github.com/apache/tvm/pull/12797


-- 
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-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] Hzfengsy commented on a diff in pull request #12797: [TIR] Construct the inverse in SuggestIndexMap

Posted by GitBox <gi...@apache.org>.
Hzfengsy commented on code in PR #12797:
URL: https://github.com/apache/tvm/pull/12797#discussion_r972550576


##########
src/tir/schedule/analysis/layout.cc:
##########
@@ -167,20 +167,25 @@ Optional<IndexMap> SuggestIndexMap(const Buffer& buffer, const Array<PrimExpr>&
     }
     return a.lower_factor > b.lower_factor;
   });
+  // Compute the inverse permutation by argsort
+  std::vector<int> inverse_order = order;
+  std::sort(inverse_order.begin(), inverse_order.end(),
+            [&order](int _a, int _b) -> bool { return order[_a] < order[_b]; });
   // Step 5. Create the indexing mapping
   auto f_alter_layout = [f_flatten_index = std::move(f_flatten_index),  //
-                         split_exprs = std::move(split_exprs),          //
-                         order = std::move(order),                      //
-                         shape = buffer->shape,                         //
+                         &split_exprs,                                  //
+                         &order,                                        //
+                             & shape = buffer->shape,                   //

Review Comment:
   ```suggestion
                           & shape = buffer->shape,                        //
   ```



-- 
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-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] vinx13 commented on a diff in pull request #12797: [TIR] Construct the inverse in SuggestIndexMap

Posted by GitBox <gi...@apache.org>.
vinx13 commented on code in PR #12797:
URL: https://github.com/apache/tvm/pull/12797#discussion_r973214730


##########
src/tir/schedule/analysis/layout.cc:
##########
@@ -167,20 +167,25 @@ Optional<IndexMap> SuggestIndexMap(const Buffer& buffer, const Array<PrimExpr>&
     }
     return a.lower_factor > b.lower_factor;
   });
+  // Compute the inverse permutation by argsort
+  std::vector<int> inverse_order = order;
+  std::sort(inverse_order.begin(), inverse_order.end(),
+            [&order](int _a, int _b) -> bool { return order[_a] < order[_b]; });
   // Step 5. Create the indexing mapping
   auto f_alter_layout = [f_flatten_index = std::move(f_flatten_index),  //
-                         split_exprs = std::move(split_exprs),          //
-                         order = std::move(order),                      //
-                         shape = buffer->shape,                         //
+                         &split_exprs,                                  //
+                         &order,                                        //
+                             & shape = buffer->shape,                   //

Review Comment:
   clang format somehow prefers this way



-- 
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-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] junrushao commented on a diff in pull request #12797: [TIR] Construct the inverse in SuggestIndexMap

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12797:
URL: https://github.com/apache/tvm/pull/12797#discussion_r972395704


##########
include/tvm/tir/index_map.h:
##########
@@ -133,16 +145,19 @@ class IndexMapNode : public Object {
   void VisitAttrs(AttrVisitor* v) {
     v->Visit("initial_indices", &initial_indices);
     v->Visit("final_indices", &final_indices);
+    v->Visit("inverse_index_map", &inverse_index_map);
   }
 
   bool SEqualReduce(const IndexMapNode* other, SEqualReducer equal) const {
     return equal.DefEqual(initial_indices, other->initial_indices) &&
-           equal(final_indices, other->final_indices);
+           equal(final_indices, other->final_indices) &&
+           equal(inverse_index_map, other->inverse_index_map);

Review Comment:
   Do we need to include `inverse_index_map` in structural equality check? Consider a case where this field is optional so `lhs` doesn't have it while `rhs` does, and should they be considered the same?



-- 
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-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] junrushao commented on a diff in pull request #12797: [TIR] Construct the inverse in SuggestIndexMap

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12797:
URL: https://github.com/apache/tvm/pull/12797#discussion_r972396471


##########
src/tir/schedule/analysis/layout.cc:
##########
@@ -167,20 +167,25 @@ Optional<IndexMap> SuggestIndexMap(const Buffer& buffer, const Array<PrimExpr>&
     }
     return a.lower_factor > b.lower_factor;
   });
+  // Compute the inverse permutation by argsort
+  std::vector<int> inverse_order = order;

Review Comment:
   Shall we add a test for `Suggest-Index-Map` where the inverse of the mapping is not detectable?



-- 
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-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] junrushao commented on a diff in pull request #12797: [TIR] Construct the inverse in SuggestIndexMap

Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #12797:
URL: https://github.com/apache/tvm/pull/12797#discussion_r972469002


##########
tests/python/unittest/test_tir_schedule_analysis.py:
##########
@@ -101,6 +101,53 @@ def test_suggest_index_map_bijective():
     assert index_map.is_equivalent_to(expected_index_map)
 
 
+def test_suggest_index_map_winograd():
+    """use case in winograd conv where the indices are complicated"""
+    i0_0_i1_0_i2_0_i3_0_i0_1_i1_1_i2_1_i3_1_fused, i3_3_fused, i4_0, i4_1 = _make_vars(
+        "i0_0_i1_0_i2_0_i3_0_i0_1_i1_1_i2_1_i3_1_fused", "i3_3_fused", "i4_0", "i4_1"
+    )
+    eps = floordiv(i0_0_i1_0_i2_0_i3_0_i0_1_i1_1_i2_1_i3_1_fused, 336) * 2 + floordiv(
+        floormod(i0_0_i1_0_i2_0_i3_0_i0_1_i1_1_i2_1_i3_1_fused, 16), 8
+    )
+    nu = floordiv(floormod(i0_0_i1_0_i2_0_i3_0_i0_1_i1_1_i2_1_i3_1_fused, 336), 112) * 2 + floordiv(
+        floormod(i0_0_i1_0_i2_0_i3_0_i0_1_i1_1_i2_1_i3_1_fused, 8), 4
+    )
+    co = floormod(i0_0_i1_0_i2_0_i3_0_i0_1_i1_1_i2_1_i3_1_fused, 4) * 32 + i3_3_fused

Review Comment:
   😂 shall we somehow use a less cryptic name?



-- 
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-unsubscribe@tvm.apache.org

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