Skip to content

Commit

Permalink
Remove replace_all and make VTs mutable (#113725)
Browse files Browse the repository at this point in the history
Summary:
1.  Removes calls to `replace_all` and `clone` and makes VTs mutable.
2. Properly handles Tuple Iterator mutation. Previously TupleIterator variables would only be properly reconstructed if they were advanced at least once in a frame. On calls to `next`, the source information would be lost (due to constructing a new iterator without using builder), which would ensure that during codegen the variable would be reconstructed from scratch. Now that VTs are mutated, the source is never lost, so we need to properly track mutation and handle it by replaying calls to `next` at the end of the modified bytecode.
3. Added test for checking iadd side effects, this was missing in our unit test coverage.
4. Fixed two incorrect sources, DelayGraphBreakVariable, and UserMethodVariable both relied on setting the source to AttrSource(parent, name) at the callsite of `var_getattr`.
5. Fixed a bug in inplace adding for lists, it would set the resulting VariableTracker's source to `None` which would utilize a different reconstruct path in codegen. Now this is handled explicitly by reconstructing vars when allow_cache=`False`, so that during side effect replay, the mutated var is correctly updated.

In subsequent PRs:
* Refactoring side effect tracking to be significantly simpler (I think we only need an `is_modified` flag)
* Refactor `next_variables` iterator to match the signature of `next`
* Remove all references to `options` in the code
* Refactor VTs representing mutable collections to implement their own mutation update handling
* Remove clone and/or make it specific to lists for creating slices
* Add mutation tracking/replay for sets
* Add mutation tracking/replay for iter.py
* Removing setting source in builder (it's set at the top level after a var is returned)

X-link: pytorch/pytorch#113725
Approved by: https://github.com/jansel

Reviewed By: osalpekar

Differential Revision: D52017956

Pulled By: mlazos

fbshipit-source-id: d586b791e8eb9d9bebb360c75628d99d362e3961
  • Loading branch information
mlazos authored and facebook-github-bot committed Dec 12, 2023
1 parent f8f3aec commit 075f804
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions userbenchmark/dynamo/dynamobench/_dynamo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,9 @@ def tuple_iterator_getitem(it, index):
return obj[start + index]


iter_next = next


def enum_repr(value, local):
# enum class can override __str__ method. Use __class__ and name attribute
# to extract the class name and key name.
Expand Down

0 comments on commit 075f804

Please sign in to comment.