compiler can incorrectly optimize a run of stores to the same name preceded by a SWAP

If the apply_static_swaps optimization in the compiler sees the instruction sequence SWAP 2; STORE_FAST a; STORE_FAST a, it will optimize that by removing the SWAP and swapping the two instructions, resulting in STORE_FAST a; STORE_FAST a.

But of course, in this case the two instructions are identical, and their ordering matters because they store to the same location. So this change results in the wrong value being stored to a.

This was exposed by comprehension inlining, since it can result in this bytecode sequence for code in the form a = [1 for a in [0]] (where the first STORE_FAST a is restoring the previous value of a from before the comprehension, if any, and the second STORE_FAST a is storing the result of the comprehension to a.).

Linked PRs

I see two options for fixing this:

Change apply_static_swaps to also track store locations and consider instructions not swappable if they store to the same location.
Add redundant store elimination, so prior to apply_static_swaps we would reduce SWAP 2; STORE_FAST a; STORE_FAST a to SWAP_2; POP_TOP; STORE_FAST a, which apply_static_swaps would correctly optimize to STORE_FAST a; POP_TOP.

Probably the ideal is to do both; (2) because it results in the best compiler output, and (1) because it is more robust and avoids implicit dependency of one optimization on another.

This is definitely a bug in apply_static_swaps:

3.10:

>>> def f(x, y):
...     a, a = x, y
...     return a
... 
>>> f(True, False)
False

3.11:

>>> def f(x, y):
...     a, a = x, y
...     return a
... 
>>> f(True, False)
True

pythongh-104615: don't make unsafe swaps in apply_static_swaps

gh-104615: don't make unsafe swaps in apply_static_swaps (#104620)

I'm considering this issue fixed by the merged PR. I filed #104635 for the separate question of improving compiler output in these cases with dead store elimination.

Merge branch 'main' into nestedcomps

[3.11] pythongh-104615: don't make unsafe swaps in apply_static_swaps (…

[3.11] gh-104615: don't make unsafe swaps in apply_static_swaps (GH-1…

Navigation Menu

Repository navigation

compiler can incorrectly optimize a run of stores to the same name preceded by a SWAP #104615

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

compiler can incorrectly optimize a run of stores to the same name preceded by a SWAP #104615

Description

Linked PRs

Activity

carljm commented on May 18, 2023

brandtbucher commented on May 18, 2023

carljm commented on May 18, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions