[jit] fix tuple index and slicing #21450
Conversation
Previously, we were not correctly normalizing slices/indexes.
zdevito
left a comment
zdevito
left a comment
There was a problem hiding this comment.
I think tuple indices should be normalize before they ever make it into the Graph. TupleSlice should assert that this is true when created. As the patch is currently is written it forces the entire compiler to know about the more complicated behavior, but tuple indices are always constant and can be normalized on creation.
|
yeah I came to the same realization lol. Will put up the new version soon |
| auto tuple = n->inputs().at(0); | ||
| const size_t tuple_len = tuple->type()->containedTypes().size(); | ||
| const size_t normalized_idx = normalizeIndex(*maybe_int, tuple_len); | ||
| n->output()->replaceAllUsesWith(construct->inputs().at(normalized_idx)); |
There was a problem hiding this comment.
I think this is the only change needed, but it's not safe to use the normalized_idx since it maybe out of bounds. I don't think tupleslice has a problem.
|
Can you land this or do you mind if I submit a PR to fix it ? Would like to get this fixed asap |
|
I'll fix it up and land |
|
Closing in favor of a PR @eellison will put up |
Stack from ghstack:
Previously, we were not correctly normalizing slices/indexes.
Closes https://github.com/pytorch/lockdown/issues/102