[jit] Consider contained types in alias analysis #21431
Closed
Conversation
This file contains hidden or 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
Container types now track the aliasing information of their contained elements. Creating a pointer from one container to another should correctly create a pointer between their contained types as well. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` I think this change also makes it so that we can be more explicit with schema annotations (wildcards can become implementation details finally!) A follow-on PR will try to make that happen
pytorchbot
added
oncall: jit
This was referenced Jun 5, 2019
Member
Author
|
actually this is not ready for review yet |
[jit] Track contained elements in alias analysis. Container types now track the aliasing information of their contained elements. Creating a pointer from one container to another should correctly create a pointer between their contained types as well. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` I think this change also makes it so that we can be more explicit with schema annotations (wildcards can become implementation details finally!) A follow-on PR will try to make that happen gh-metadata: pytorch pytorch 21431 gh/suo/53/head
[jit] Track contained elements in alias analysis. Container types now track the aliasing information of their contained elements. Creating a pointer from one container to another should correctly create a pointer between their contained types as well. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` I think this change also makes it so that we can be more explicit with schema annotations (wildcards can become implementation details finally!) A follow-on PR will try to make that happen gh-metadata: pytorch pytorch 21431 gh/suo/53/head
[jit] Track contained elements in alias analysis. Container types now track the aliasing information of their contained elements. Creating a pointer from one container to another should correctly create a pointer between their contained types as well. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` I think this change also makes it so that we can be more explicit with schema annotations (wildcards can become implementation details finally!) A follow-on PR will try to make that happen gh-metadata: pytorch pytorch 21431 gh/suo/53/head
[jit] Track contained elements in alias analysis. Container types now track the aliasing information of their contained elements. Creating a pointer from one container to another should correctly create a pointer between their contained types as well. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` I think this change also makes it so that we can be more explicit with schema annotations (wildcards can become implementation details finally!) A follow-on PR will try to make that happen gh-metadata: pytorch pytorch 21431 gh/suo/53/head
This was referenced Jun 6, 2019
Contributor
|
I am going to need a refresher on our alias analysis pass before I have enough context to review for correctness. |
[jit] Consider contained types in alias analysis `AliasDb::getReads()` now considers contained types to be part of the read set. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` This can be improved (e.g. we can track tuple contained aliasing precisely) but I will defer that to the future. gh-metadata: pytorch pytorch 21431 gh/suo/53/head
suo
changed the title
Member
Author
|
@zdevito: Changed this as we discussed offline. Not doing any of the container elements stuff—just adding the appropriate wildcards to the |
[jit] Consider contained types in alias analysis `AliasDb::getReads()` now considers contained types to be part of the read set. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` This can be improved (e.g. we can track tuple contained aliasing precisely) but I will defer that to the future. gh-metadata: pytorch pytorch 21431 gh/suo/53/head
zdevito
approved these changes
Jun 9, 2019
[jit] Consider contained types in alias analysis `AliasDb::getReads()` now considers contained types to be part of the read set. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` This can be improved (e.g. we can track tuple contained aliasing precisely) but I will defer that to the future. gh-metadata: pytorch pytorch 21431 gh/suo/53/head
[jit] Consider contained types in alias analysis `AliasDb::getReads()` now considers contained types to be part of the read set. This is necessary because we need to consider reads of List[Tensor] to be reads both of the outer List and the inner Tensor. Otherwise something like this might happen: ``` l = [...] l[0].add_(...) print(l) # could reorder this read across the write above ``` This can be improved (e.g. we can track tuple contained aliasing precisely) but I will defer that to the future. gh-metadata: pytorch pytorch 21431 gh/suo/53/head
Contributor
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.
Stack from ghstack:
AliasDb::getReads()now considers contained types to be part of theread set.
This is necessary because we need to consider reads of List[Tensor] to
be reads both of the outer List and the inner Tensor. Otherwise
something like this might happen:
This can be improved (e.g. we can track tuple contained aliasing
precisely) but I will defer that to the future.
gh-metadata: pytorch pytorch 21431 gh/suo/53/head
Differential Revision: D15718560