[LowerVTCMAlloc] Move LowerVtcmAlloc to after StorageRewrite #12364
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
August 10, 2022 13:14
Vtcm allocations were being moved inside loops even if they were originally allocated outside of the loops. Normally PlanAndUpdateBufferAllocationLocation moves allocations as close to use as possible and then StorageRewrite moves them back out as far as possible. However, with Vtcm allocation, PlanAndUpdateBufferAllocationLocation would move the Vtcm allocation close to the compute, then LowerVtcm would convert the allocation to a LetStmt. StorageRewrite would not move this LetStmt as it only handles allocations. Moving LowerVtcmAlloc to after StorageRewrite ensures that the vtcm allocations are in their final spot before converting them to a LetStmt.
csullivan
approved these changes
Aug 12, 2022
There was a problem hiding this comment.
LGTM, thanks for the discussions and effort on this @tkonolige and @Lunderberg.
| @@ -583,8 +583,10 @@ class StoragePlanRewriter : public StmtExprMutator { | |||
| }; | |||
|
|
|||
| // Checks whether the storage_scope is especially tagged for a specific memory. | |||
| // Special memory is all combined into a single allocation. | |||
There was a problem hiding this comment.
I've verified that with the special tagged memory helper change we are getting correct behavior for aligned access in the cases that I mentioned to fail previously.
xinetzone
pushed a commit
to daobook/tvm
that referenced
this pull request
Nov 25, 2022
…12364) * [LowerVTCMAlloc] Move LowerVtcmAlloc to after StorageRewrite Vtcm allocations were being moved inside loops even if they were originally allocated outside of the loops. Normally PlanAndUpdateBufferAllocationLocation moves allocations as close to use as possible and then StorageRewrite moves them back out as far as possible. However, with Vtcm allocation, PlanAndUpdateBufferAllocationLocation would move the Vtcm allocation close to the compute, then LowerVtcm would convert the allocation to a LetStmt. StorageRewrite would not move this LetStmt as it only handles allocations. Moving LowerVtcmAlloc to after StorageRewrite ensures that the vtcm allocations are in their final spot before converting them to a LetStmt. * fix issues with tagging and storage rewrite
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Vtcm allocations were being moved inside loops even if they were originally allocated outside of the loops. Normally PlanAndUpdateBufferAllocationLocation moves allocations as close to use as possible and then StorageRewrite moves them back out as far as possible. However, with Vtcm allocation, PlanAndUpdateBufferAllocationLocation would move the Vtcm allocation close to the compute, then LowerVtcm would convert the allocation to a LetStmt. StorageRewrite would not move this LetStmt as it only handles allocations. Moving LowerVtcmAlloc to after StorageRewrite ensures that the vtcm allocations are in their final spot before converting them to a LetStmt.
@csullivan @Lunderberg