[Python-ideas] Disable all peephole optimizations

[Nick Coghlan]

On 22 May 2014 00:24, "Ned Batchelder" <ned at nedbatchelder.com> wrote:
>
> On 5/21/14 8:21 AM, Jonas Wielicki wrote:
>>
>> On 21.05.2014 14:13, Steven D'Aprano wrote:
>>>
>>> > On Wed, May 21, 2014 at 07:05:49AM -0400, Ned Batchelder wrote:
>>>>
>>>> >> ** The problem
>>>> >>
>>>> >> A long-standing problem with CPython is that the peephole optimizer
>>>> >> cannot be completely disabled.  Normally, peephole optimization is
a
>>>> >> good thing, it improves execution speed.  But in some situations,
like
>>>> >> coverage testing, it's more important to be able to reason about
the
>>>> >> code's execution.  I propose that we add a way to completely
disable the
>>>> >> optimizer.
>>>
>>> >
>>> > I'm not sure whether this is an argument for or against your
proposal,
>>> > but the continue statement shown below is *not* dead code and should
not
>>> > be optimized out. The assert fails if you remove the continue
statement.
>>> >
>>> > I don't have 3.4 on this machine to test with, but using 3.3, I can
see
>>> > no evidence that `continue` is optimized away.
>>
>> The logical continue is still there -- what happens is that the
>> optimizer rewrites the `else` jump at the preceding `if` condition,
>> which would normally point at the `continue` statement, to the beginning
>> of the loop, because it would be a jump (to the continue) to a jump (to
>> the for loop header).
>>
>> Thus, the actual continue statement is not reached, but logically the
>> code does the same, because the only way continue would have been
>> reached was transformed to a continue itself.
>>
> To make the details more explicit, here is the source again, and the
disassembled code, with the original source interspersed:
>
>> a = b = c = 0
>> for n in range(100):
>>     if n % 2:
>>         if n % 4:
>>             a += 1
>>         continue
>>     else:
>>         b += 1
>>     c += 1
>> assert a == 50 and b == 50 and c == 50
>
> Disassembled (Python 3.4, but the same effect is visible in 2.7, 3.3,
etc):
>
>
> a = b = c = 0
>   1           0 LOAD_CONST               0 (0)
>               3 DUP_TOP
>               4 STORE_NAME               0 (a)
>               7 DUP_TOP
>               8 STORE_NAME               1 (b)
>              11 STORE_NAME               2 (c)
>
> for n in range(100):
>   2          14 SETUP_LOOP              79 (to 96)
>              17 LOAD_NAME                3 (range)
>              20 LOAD_CONST               1 (100)
>              23 CALL_FUNCTION            1 (1 positional, 0 keyword pair)
>              26 GET_ITER
>         >>   27 FOR_ITER                65 (to 95)
>              30 STORE_NAME               4 (n)
>
>     if n % 2:
>   3          33 LOAD_NAME                4 (n)
>              36 LOAD_CONST               2 (2)
>              39 BINARY_MODULO
>              40 POP_JUMP_IF_FALSE       72
>
>         if n % 4:
>   4          43 LOAD_NAME                4 (n)
>              46 LOAD_CONST               3 (4)
>              49 BINARY_MODULO
>              50 POP_JUMP_IF_FALSE       27
>
>             a += 1
>   5          53 LOAD_NAME                0 (a)
>              56 LOAD_CONST               4 (1)
>              59 INPLACE_ADD
>              60 STORE_NAME               0 (a)
>              63 JUMP_ABSOLUTE           27
>
>         continue
>   6          66 JUMP_ABSOLUTE           27
>              69 JUMP_FORWARD            10 (to 82)
>
>         b += 1
>   8     >>   72 LOAD_NAME                1 (b)
>              75 LOAD_CONST               4 (1)
>              78 INPLACE_ADD
>              79 STORE_NAME               1 (b)
>
>     c += 1
>   9     >>   82 LOAD_NAME                2 (c)
>              85 LOAD_CONST               4 (1)
>              88 INPLACE_ADD
>              89 STORE_NAME               2 (c)
>              92 JUMP_ABSOLUTE           27
>         >>   95 POP_BLOCK
>
>
> assert a == 50 and b == 50 and c == 50
>  10     >>   96 LOAD_NAME                0 (a)
>              99 LOAD_CONST               5 (50)
>             102 COMPARE_OP               2 (==)
>             105 POP_JUMP_IF_FALSE      132
>             108 LOAD_NAME                1 (b)
>             111 LOAD_CONST               5 (50)
>             114 COMPARE_OP               2 (==)
>             117 POP_JUMP_IF_FALSE      132
>             120 LOAD_NAME                2 (c)
>             123 LOAD_CONST               5 (50)
>             126 COMPARE_OP               2 (==)
>             129 POP_JUMP_IF_TRUE       138
>         >>  132 LOAD_GLOBAL              5 (AssertionError)
>             135 RAISE_VARARGS            1
>         >>  138 LOAD_CONST               6 (None)
>             141 RETURN_VALUE
>
> Notice that line 6 (the continue) is unreachable, because the else-jump
from line 4 has been turned into a jump to bytecode offset 27 (the for
loop), and the end of line 5 has also been turned into a jump to 27, rather
than letting it flow to line 6.   So line 6 still exists in the bytecode,
but is never executed, leading tracing tools to indicate that line 6 is
never executed.

So isn't this just a bug in the dead code elimination? Fixing that (so
there's no bytecode behind that line and coverage tools can know it has
been optimised out) sounds better than adding an obscure config option.

Potentially less risky would be to provide a utility in the dis module to
flag such lines after the fact.

Cheers,
Nick.


>
> --Ned.
>
>
> _______________________________________________
> Python-ideas mailing list
> Python-ideas at python.org
> https://mail.python.org/mailman/listinfo/python-ideas
> Code of Conduct: http://python.org/psf/codeofconduct/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-ideas/attachments/20140522/2928787d/attachment.html>