You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "yelite (via GitHub)" <gi...@apache.org> on 2023/06/16 15:58:45 UTC

[GitHub] [tvm] yelite opened a new pull request, #15118: [Unity] Allocate workspace for all functions

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

   This PR makes `AllocateWorkspace` allocates workspace to all Relax functions that doesn't have `Codegen` or `Composite` attributes.
   
   cc @masahi 


-- 
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] tvm-bot commented on pull request #15118: [Unity] Allocate workspace for all functions

Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #15118:
URL: https://github.com/apache/tvm/pull/15118#issuecomment-1594910701

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-ccs-start-->
    * cc @quic-sanirudh <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


-- 
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] yelite commented on pull request #15118: [Unity] Allocate workspace for all functions

Posted by "yelite (via GitHub)" <gi...@apache.org>.
yelite commented on PR #15118:
URL: https://github.com/apache/tvm/pull/15118#issuecomment-1597303634

   @masahi I think what you described is ideal and the workspace param can be made as `R.Object` to workaround the multi-caller limitation. I am just not sure what we lose by stripping out shape information from the workspace param. If all the usages of workspace validates the actual size of workspace, and allocate if workspace is not large enough, like what attention does currently, I think it should be fine.


-- 
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] masahi commented on pull request #15118: [Unity] Allocate workspace for all functions

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi commented on PR #15118:
URL: https://github.com/apache/tvm/pull/15118#issuecomment-1596615802

   This assumes that all entry functions get the same workspace size right?
   
   I also have a WIP branch https://github.com/masahi/tvm/tree/workspace-provider-entry-func that calculates the minimum-necessary workspace size for each function that calls another functions with `kWorkSpaceSize` attribute. It doesn't yet handle the case where the same function with `kWorkSpaceSize` is called by multiple entry functions, with different maximum workspace sizes. Since we modify the function signature for a function that needs workspace param, the param size needs to take into account all entry funcs that call it.    
   
   So the upshot is, even though different entry funcs can have its own minimum workspace size, to handle the case above we need to take the max over all functions with `kWorkSpaceSize`, even though a single entry func might not call all of them. I was planning to add such logic on top of my branch, but looking at this PR makes me realize that this simple patch is effectively sufficient.
   
   Do you agree with my assessment? @yelite  


-- 
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] masahi merged pull request #15118: [Unity] Allocate workspace for all functions

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi merged PR #15118:
URL: https://github.com/apache/tvm/pull/15118


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