◐ Shell
clean mode source ↗

gh-126835: make CFG optimizer skip over NOP's when looking for const sequence construction by WolframAlph · Pull Request #129703 · python/cpython

We need to account for optimizations interfering with each other when moving more and more AST optimizations to CFG. Currently is_constant_sequence & (fold_tuple_on_constants & optimize_if_const_list_or_set that use it) do not take into account NOPs in between. This becomes problematic when there are several optimizations performed one after another as they NOP out unused instructions. Example:

This expression will trigger subscript & binop foldings. With current is_constant_sequence implementation, resulting instruction sequence cannot be optimized correctly. Here is final dis:

  1           RESUME                   0

  2           LOAD_SMALL_INT           1
              LOAD_CONST               1 ((3,))
              BUILD_TUPLE              2
              LOAD_SMALL_INT           1
              BINARY_SUBSCR
              LOAD_SMALL_INT           0
              BINARY_SUBSCR
              RETURN_VALUE

When correct sequence should be:

  1           RESUME                   0

  2           LOAD_SMALL_INT           3
              RETURN_VALUE

Here is basic block dump in a state when BUILD_TUPLE fails to be optimized by fold_tuple_on_constants:

Screenshot 2025-02-05 at 21 47 58

As you can see, BUILD_TUPLE 2 has 2 consts before it but they are separated by NOPs due to previous binop folding. Obvious solution would be to remove NOPs on every iteration in optimize_basic_block which makes sense, but we cannot do that because we would be changing basic block size while iterating over it which breaks everything. So a different approach should be implemented. This PR presents one of the possible approaches to prepare us for the next optimizations migration to CFG. My main concern is complexity & readability as I think they suffer a bit from it. Another cleaner way I see is to use PyMem_Malloc to store indexes of corresponding instructions which seems cleaner but should be discussed.

cc @Eclips4 @tomasr8 @iritkatriel