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/04/26 09:15:35 UTC
[GitHub] [tvm] Hzfengsy opened a new pull request #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Hzfengsy opened a new pull request #7923:
URL: https://github.com/apache/tvm/pull/7923
This PR is part of the TensorIR upstreaming effort (#7527) as one of the M1c stages, introducing the pass `ConvertBlocksToOpaque` and `CompactBufferAllocation`
Pass `ConvertBlocksToOpaque` will substitute all the block vars with the PrimExprs they are bound to its iter_values and convert blocks into opaque ones.
Pass `CompactBufferAllocation` compacts the buffer access region by removing the buffer regions that are not accessed.
These two passes both are important passes for converting schedulable TensorIR to low-level tir.
cc @tqchen @junrushao1994 @comaniac @xqdan @jroesch
cc @tqchen @comaniac @jroesch @junrushao1994
--
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] Hzfengsy commented on a change in pull request #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
Hzfengsy commented on a change in pull request #7923:
URL: https://github.com/apache/tvm/pull/7923#discussion_r620755304
##########
File path: include/tvm/tir/stmt.h
##########
@@ -991,13 +992,22 @@ class BufferRegion : public ObjectRef {
TVM_DLL explicit BufferRegion(Buffer buffer, Array<Range> region);
/*!
- * \brief Create a BufferRegion which is full region of the given buffer..
+ * \brief Create a BufferRegion which is full region of the given buffer.
* \param buffer The buffer to generate full BufferRegion.
* \return The BufferRegion which covers all region of the given buffer
*/
TVM_DLL static BufferRegion FullRegion(Buffer buffer);
+ /*!
+ * \brief Create a BufferRegion which is a single point of the given buffer.
+ * \param buffer The buffer to generate single point BufferRegion.
+ * \param indices The access point indices of the buffer
+ * \return The BufferRegion which is the single point of the given buffer.
+ */
+ TVM_DLL static BufferRegion FromPoint(Buffer buffer, Array<PrimExpr> indices);
Review comment:
`FromRegion` is not accurate. I guess `FullRegion` is good or we can call it `FromFullRegion`
--
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] xqdan commented on a change in pull request #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
xqdan commented on a change in pull request #7923:
URL: https://github.com/apache/tvm/pull/7923#discussion_r620763914
##########
File path: python/tvm/tir/transform/transform.py
##########
@@ -560,3 +560,53 @@ def PlanAndUpdateBufferAllocationLocation():
The result pass
"""
return _ffi_api.PlanAndUpdateBufferAllocationLocation()
+
+
+def ConvertBlocksToOpaque():
+ """Substitute all the block vars with the PrimExprs they are bound to, indicated by
+ the corresponding iter_values in BlockRealize, and then convert the blocks into
+ opaque ones by removing all the iter_values in BlockRealize and iter_vars in Block.
+
+ Returns
+ -------
+ fpass : tvm.transform.Pass
+ The result pass
+ """
+ return _ffi_api.ConvertBlocksToOpaque()
+
+
+def CompactBufferAllocation():
Review comment:
So，is this pass doing the same job as InferBound?
--
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 pull request #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
jcf94 commented on pull request #7923:
URL: https://github.com/apache/tvm/pull/7923#issuecomment-828148762
Merged. Thanks! @Hzfengsy @tqchen @comaniac @junrushao1994 @xqdan
--
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 #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7923:
URL: https://github.com/apache/tvm/pull/7923#discussion_r620643414
##########
File path: include/tvm/tir/transform.h
##########
@@ -360,6 +360,52 @@ TVM_DLL Pass LowerInitBlock();
*/
TVM_DLL Pass PlanAndUpdateBufferAllocationLocation();
+/*!
+ * \brief Substitute all the block vars with the PrimExprs they are bound to, indicated by the
+ * corresponding iter_values in BlockRealize, and then convert the blocks into opaque
+ * ones by removing all the iter_values in BlockRealize and iter_vars in Block.
Review comment:
Substitute ... for, or Convert ... to.
```suggestion
* \brief Substitute all the block vars with the PrimExprs they are bound to, indicated by the
* corresponding iter_values in BlockRealize, for opaque blocks by removing all
*. the iter_values in BlockRealize and iter_vars in Block.
```
##########
File path: include/tvm/tir/stmt.h
##########
@@ -991,13 +992,22 @@ class BufferRegion : public ObjectRef {
TVM_DLL explicit BufferRegion(Buffer buffer, Array<Range> region);
/*!
- * \brief Create a BufferRegion which is full region of the given buffer..
+ * \brief Create a BufferRegion which is full region of the given buffer.
* \param buffer The buffer to generate full BufferRegion.
* \return The BufferRegion which covers all region of the given buffer
*/
TVM_DLL static BufferRegion FullRegion(Buffer buffer);
+ /*!
+ * \brief Create a BufferRegion which is a single point of the given buffer.
+ * \param buffer The buffer to generate single point BufferRegion.
+ * \param indices The access point indices of the buffer
+ * \return The BufferRegion which is the single point of the given buffer.
+ */
+ TVM_DLL static BufferRegion FromPoint(Buffer buffer, Array<PrimExpr> indices);
Review comment:
Given this naming convension, should `FullRegion` be renamed to `FromRegion`?
--
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 merged pull request #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
jcf94 merged pull request #7923:
URL: https://github.com/apache/tvm/pull/7923
--
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] xqdan commented on a change in pull request #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
xqdan commented on a change in pull request #7923:
URL: https://github.com/apache/tvm/pull/7923#discussion_r621737814
##########
File path: python/tvm/tir/transform/transform.py
##########
@@ -560,3 +560,53 @@ def PlanAndUpdateBufferAllocationLocation():
The result pass
"""
return _ffi_api.PlanAndUpdateBufferAllocationLocation()
+
+
+def ConvertBlocksToOpaque():
+ """Substitute all the block vars with the PrimExprs they are bound to, indicated by
+ the corresponding iter_values in BlockRealize, and then convert the blocks into
+ opaque ones by removing all the iter_values in BlockRealize and iter_vars in Block.
+
+ Returns
+ -------
+ fpass : tvm.transform.Pass
+ The result pass
+ """
+ return _ffi_api.ConvertBlocksToOpaque()
+
+
+def CompactBufferAllocation():
Review comment:
Thanks for explaination!
--
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 #7923: [TensorIR][PASS][M1c] CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7923:
URL: https://github.com/apache/tvm/pull/7923#discussion_r621136104
##########
File path: python/tvm/tir/transform/transform.py
##########
@@ -560,3 +560,53 @@ def PlanAndUpdateBufferAllocationLocation():
The result pass
"""
return _ffi_api.PlanAndUpdateBufferAllocationLocation()
+
+
+def ConvertBlocksToOpaque():
+ """Substitute all the block vars with the PrimExprs they are bound to, indicated by
+ the corresponding iter_values in BlockRealize, and then convert the blocks into
+ opaque ones by removing all the iter_values in BlockRealize and iter_vars in Block.
+
+ Returns
+ -------
+ fpass : tvm.transform.Pass
+ The result pass
+ """
+ return _ffi_api.ConvertBlocksToOpaque()
+
+
+def CompactBufferAllocation():
Review comment:
Yes, the functionality should be similar, except that infer bound needs to walk through the schedule tree, while in this case the split/reorder already updated the index, so the impl should be simpler
--
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