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 2021/06/01 17:18:28 UTC

[GitHub] [tvm] junrushao1994 opened a new pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

junrushao1994 opened a new pull request #8170:
URL: https://github.com/apache/tvm/pull/8170


   This PR is part of the TensorIR upstreaming effort (#7527), which adds the first 2 schedule primitives:
   - compute-Inline
   - reverse-compute-inline
   


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

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



[GitHub] [tvm] junrushao1994 commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643420194



##########
File path: src/tir/schedule/analysis.h
##########
@@ -41,8 +41,51 @@ void VerifySRefTree(const ScheduleState& self);
  */
 void VerifyCachedFlags(const ScheduleState& self);
 
-/******** Binding ********/
+/******** Scope ********/
+/*!
+ * \brief Get the sref to the scope root block, exclusive
+ * \param sref The block or loop sref to be retrieved
+ * \return The sref to the scope root block. NullOpt if `sref` is the root block of the IR
+ */
+Optional<StmtSRef> GetScopeRoot(const StmtSRef& sref);
+
+/*!
+ * \brief Checks if scope the specified sref is in is a stage-pipeline and return it

Review comment:
       @tkonolige once told me that we should use "Checks" (IIRC), but I agree we should append "if so" to the end of the sentence




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

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



[GitHub] [tvm] junrushao1994 merged pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #8170:
URL: https://github.com/apache/tvm/pull/8170


   


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

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



[GitHub] [tvm] jcf94 commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643608174



##########
File path: src/support/array.h
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SUPPORT_ARRAY_H_
+#define TVM_SUPPORT_ARRAY_H_
+#include <tvm/runtime/container.h>
+
+#include <vector>
+
+namespace tvm {
+namespace support {
+
+/*!
+ * \brief Checks if two arrays contain the same objects
+ * \tparam T The type of objects in the array
+ * \param a The first array
+ * \param b The second array
+ * \return A boolean indicating if they are the same
+ */
+template <class T>
+inline bool ArraySame(const Array<T>& a, const Array<T>& b) {

Review comment:
       How about to merge this as a member function of `Array`? Some object of TVM has member functions like `A.same_as(B)`.




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

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



[GitHub] [tvm] junrushao1994 merged pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #8170:
URL: https://github.com/apache/tvm/pull/8170


   


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

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



[GitHub] [tvm] comaniac commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643386341



##########
File path: src/support/array.h
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SUPPORT_ARRAY_H_
+#define TVM_SUPPORT_ARRAY_H_
+#include <tvm/runtime/container.h>
+
+#include <vector>
+
+namespace tvm {
+namespace support {
+
+/*!
+ * \brief Checks if two arrays contain the same objects
+ * \tparam T The type of objects in the array
+ * \param a The first array
+ * \param b The second array
+ * \return A boolean indicating if they are the same
+ */
+template <class T>
+inline bool ArraySame(const Array<T>& a, const Array<T>& b) {

Review comment:
       Is this following a naming convention? Otherwise `SameArray` or `IsSameArray` seem more straightforward.

##########
File path: src/tir/schedule/utils.h
##########
@@ -114,6 +116,33 @@ inline bool CanRelaxStorageUndereThread(const runtime::StorageScope& storage_sco
   return static_cast<int>(storage_scope.rank) <= static_cast<int>(thread_scope.rank);
 }
 
+/******** SeqStmt ********/
+
+/*!
+ * \brief Remove a specific Stmt from a SeqStmt. If a SeqStmt contains a BlockRealize,
+ * whose block is the Stmt to be removed, then remove that BlockRealize too.
+ * \param seq The SeqStmt to be removed from
+ * \param to_remove The Stmt to be removed
+ * \return The removal result
+ */
+inline Stmt SeqStmtRemove(const SeqStmt& seq, const Stmt& to_remove) {

Review comment:
       Ditto. `RemoveSeqStmt`?

##########
File path: src/tir/schedule/analysis.h
##########
@@ -41,8 +41,51 @@ void VerifySRefTree(const ScheduleState& self);
  */
 void VerifyCachedFlags(const ScheduleState& self);
 
-/******** Binding ********/
+/******** Scope ********/
+/*!
+ * \brief Get the sref to the scope root block, exclusive
+ * \param sref The block or loop sref to be retrieved
+ * \return The sref to the scope root block. NullOpt if `sref` is the root block of the IR
+ */
+Optional<StmtSRef> GetScopeRoot(const StmtSRef& sref);
+
+/*!
+ * \brief Checks if scope the specified sref is in is a stage-pipeline and return it

Review comment:
       ```suggestion
    * \brief Check if scope the specified sref is in is a stage-pipeline and return it if so
   ```




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

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



[GitHub] [tvm] junrushao1994 commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643418939



##########
File path: src/support/array.h
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SUPPORT_ARRAY_H_
+#define TVM_SUPPORT_ARRAY_H_
+#include <tvm/runtime/container.h>
+
+#include <vector>
+
+namespace tvm {
+namespace support {
+
+/*!
+ * \brief Checks if two arrays contain the same objects
+ * \tparam T The type of objects in the array
+ * \param a The first array
+ * \param b The second array
+ * \return A boolean indicating if they are the same
+ */
+template <class T>
+inline bool ArraySame(const Array<T>& a, const Array<T>& b) {

Review comment:
       Thanks for the suggestion! I will go with `IsSameArray`




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

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



[GitHub] [tvm] junrushao1994 commented on pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#issuecomment-854174452


   Hey sorry for the late response! I addressed the comments accordingly. Please take another look :-)


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

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



[GitHub] [tvm] junrushao1994 commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r644147041



##########
File path: src/support/array.h
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SUPPORT_ARRAY_H_
+#define TVM_SUPPORT_ARRAY_H_
+#include <tvm/runtime/container.h>
+
+#include <vector>
+
+namespace tvm {
+namespace support {
+
+/*!
+ * \brief Checks if two arrays contain the same objects
+ * \tparam T The type of objects in the array
+ * \param a The first array
+ * \param b The second array
+ * \return A boolean indicating if they are the same
+ */
+template <class T>
+inline bool ArraySame(const Array<T>& a, const Array<T>& b) {

Review comment:
       Sounds good. I will go with ArrayWithSameContent




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

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



[GitHub] [tvm] tqchen commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643923522



##########
File path: src/support/array.h
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SUPPORT_ARRAY_H_
+#define TVM_SUPPORT_ARRAY_H_
+#include <tvm/runtime/container.h>
+
+#include <vector>
+
+namespace tvm {
+namespace support {
+
+/*!
+ * \brief Checks if two arrays contain the same objects
+ * \tparam T The type of objects in the array
+ * \param a The first array
+ * \param b The second array
+ * \return A boolean indicating if they are the same
+ */
+template <class T>
+inline bool ArraySame(const Array<T>& a, const Array<T>& b) {

Review comment:
       same_as is a shallow ptr check which is different. ArrayWithSameContents might be a good way to differentiate




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

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



[GitHub] [tvm] junrushao1994 commented on pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#issuecomment-854174452


   Hey sorry for the late response! I addressed the comments accordingly. Please take another look :-)


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

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



[GitHub] [tvm] comaniac commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643427302



##########
File path: src/tir/schedule/analysis.h
##########
@@ -41,8 +41,51 @@ void VerifySRefTree(const ScheduleState& self);
  */
 void VerifyCachedFlags(const ScheduleState& self);
 
-/******** Binding ********/
+/******** Scope ********/
+/*!
+ * \brief Get the sref to the scope root block, exclusive
+ * \param sref The block or loop sref to be retrieved
+ * \return The sref to the scope root block. NullOpt if `sref` is the root block of the IR
+ */
+Optional<StmtSRef> GetScopeRoot(const StmtSRef& sref);
+
+/*!
+ * \brief Checks if scope the specified sref is in is a stage-pipeline and return it

Review comment:
       Actually both are fine to me as long as it is consistent. For example, the above `GetScopeRoot` has "Get the sref...". It should be "Gets the sref" then.

##########
File path: src/tir/schedule/utils.h
##########
@@ -114,6 +116,33 @@ inline bool CanRelaxStorageUndereThread(const runtime::StorageScope& storage_sco
   return static_cast<int>(storage_scope.rank) <= static_cast<int>(thread_scope.rank);
 }
 
+/******** SeqStmt ********/
+
+/*!
+ * \brief Remove a specific Stmt from a SeqStmt. If a SeqStmt contains a BlockRealize,
+ * whose block is the Stmt to be removed, then remove that BlockRealize too.
+ * \param seq The SeqStmt to be removed from
+ * \param to_remove The Stmt to be removed
+ * \return The removal result
+ */
+inline Stmt SeqStmtRemove(const SeqStmt& seq, const Stmt& to_remove) {

Review comment:
       Sounds good.




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

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



[GitHub] [tvm] junrushao1994 commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643498113



##########
File path: src/tir/schedule/analysis.h
##########
@@ -41,8 +41,51 @@ void VerifySRefTree(const ScheduleState& self);
  */
 void VerifyCachedFlags(const ScheduleState& self);
 
-/******** Binding ********/
+/******** Scope ********/
+/*!
+ * \brief Get the sref to the scope root block, exclusive
+ * \param sref The block or loop sref to be retrieved
+ * \return The sref to the scope root block. NullOpt if `sref` is the root block of the IR
+ */
+Optional<StmtSRef> GetScopeRoot(const StmtSRef& sref);
+
+/*!
+ * \brief Checks if scope the specified sref is in is a stage-pipeline and return it

Review comment:
       Got it. I am going to change all of them to "Gets"




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

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



[GitHub] [tvm] junrushao1994 commented on a change in pull request #8170: [TensorIR][M2a] Compute-Inline,Reverse-Compute-Inline

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8170:
URL: https://github.com/apache/tvm/pull/8170#discussion_r643419741



##########
File path: src/tir/schedule/utils.h
##########
@@ -114,6 +116,33 @@ inline bool CanRelaxStorageUndereThread(const runtime::StorageScope& storage_sco
   return static_cast<int>(storage_scope.rank) <= static_cast<int>(thread_scope.rank);
 }
 
+/******** SeqStmt ********/
+
+/*!
+ * \brief Remove a specific Stmt from a SeqStmt. If a SeqStmt contains a BlockRealize,
+ * whose block is the Stmt to be removed, then remove that BlockRealize too.
+ * \param seq The SeqStmt to be removed from
+ * \param to_remove The Stmt to be removed
+ * \return The removal result
+ */
+inline Stmt SeqStmtRemove(const SeqStmt& seq, const Stmt& to_remove) {

Review comment:
       What about `RemoveFromSeqStmt`?




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

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