gh-99360: Avoid `exc=None; del exc` in bytecode where unnecessary #99361

sweeneyde · 2022-11-11T04:14:30Z

Functional changes:

DELETE_FAST now assumes its argument is bound; it no longer raises UnboundLocalError by itself.
When the state of a local is uncertain, del x compiles to LOAD_FAST_CHECK x; POP_TOP; DELETE_FAST x
At the end of an exception handler except E as exc: ..., the exc=None before the del exc
is omitted when not needed.

Issue: Avoid exc=None; del exc in bytecode where unnecessary #99360

iritkatriel · 2022-11-15T16:43:33Z

Python/compile.c

+expand_del_noerror(basicblock *entryblock, int none_oparg)
+{
+    for (basicblock *b = entryblock; b != NULL; b = b->b_next) {
+        int room_needed = 0;


This function would be simpler if you use insert_instruction(). Is it for optimization that you aren't?

Yeah, I was trying to avoid quadratic time, but I suppose that could only happen if one long basicblock had many DELETE_FAST_CHECK instructions, e.g.

def f(x): del x; del x; del x; del x; del x del x; del x; del x; del x; del x del x; del x; del x; del x; del x

But such code is useless, so I'll try to clean this up.

The DELETE_NOERROR instructions should typically happen at basicblock boundaries, so I believe there shouldn't be any performance issues there.

iritkatriel · 2022-11-17T10:15:17Z

Python/compile.c

+                            break;
+                    }
+                    b->b_instr[i].i_opcode = del_opcode;
+                    if (insert_instruction(b, i, &(struct instr) {


Perhaps we can avoid shifting here:

in compiler_nameop, add a couple of NOPs so that there is space for this expansion. If no expansion happens, the NOPs can be removed by a final remove_redundant_nops().

optimize_cfg removes NOPs already, so we'd have to either introduce another pseudo-opcode or shuffle around assemble() a bit.

We could have a new pseudo instruction called BLANK that is ignored by the assembler (has size 0 and emits nothing).

markshannon · 2022-11-21T18:31:36Z

Is it really worthwhile extending this to global, nonlocal and class-local variables? They will be vanishingly rare.
This seems like a lot of code for a relatively minor improvement.

I thought the original idea was to change the code for del local from DELETE_FAST to LOAD_FAST_CHECK; POP_TOP; DELETE_FAST, and the code cleaning up except Exception as var: to something like STORE_FAST_NONE_IF_NULL; DELETE_FAST
The existing pass that eliminates checks can then be modified to convert STORE_FAST_NONE_IF_NULL to NOP in the same way that it converts LOAD_FAST_CHECK to LOAD_FAST.

The peepholer can convert any residual LOAD_FAST; POP_TOP to NOP.
The assembler will need to convert STORE_FAST_NONE_IF_NULL to LOAD_CONST None; STORE_FAST.

sweeneyde · 2022-11-22T08:02:54Z

Is it really worthwhile extending this to global, nonlocal and class-local variables? They will be vanishingly rare. This seems like a lot of code for a relatively minor improvement.

I agree that anything other than a local is not worth it, and the PR doesn't change compiler output except when deleting fast locals. I've updated the PR so that it's clearer that there are no changes to those other cases. Adding an extra ctx possibility into compiler_nameop was the cleanest implementation I could come up with to avoid duplicating the symtable logic.

Over half of the code added is tests and docs. The tests are checking the bytecode for using DEREF/GLOBAL/NAME for the exception name as well, since compile_nameop is implementing that deletion after this PR, rather than the callers of compile_nameop, even though there should be no change. Also, refactoring add_local_null_checks to include deletions actually makes it more understandable IMO.

I thought the original idea was to change the code for del local from DELETE_FAST to LOAD_FAST_CHECK; POP_TOP; DELETE_FAST, and the code cleaning up except Exception as var: to something like STORE_FAST_NONE_IF_NULL; DELETE_FAST The existing pass that eliminates checks can then be modified to convert STORE_FAST_NONE_IF_NULL to NOP in the same way that it converts LOAD_FAST_CHECK to LOAD_FAST.

The peepholer can convert any residual LOAD_FAST; POP_TOP to NOP. The assembler will need to convert STORE_FAST_NONE_IF_NULL to LOAD_CONST None; STORE_FAST.

The add_local_null_checks function does not convert LOAD_FAST_CHECK into LOAD_FAST, but the other way around: it switches from the unchecked version to the checked version where needed. This is easier to implement because any path through the code that leaves the local undefined is enough to switch to the unsafe version, so it's akin to

instr.checked = any(p.unsafe(local_i) for p in codepaths_to_instr)

Assuming this approach, there's a need for the dataflow analysis to turn NOP; DELETE_FAST into STORE_FAST_NONE_IF_NULL; DELETE_FAST. But then DELETE_FAST would have to turn into LOAD_FAST_CHECK;POP_TOP;DELETE_FAST or LOAD_CONST(None);STORE_FAST;DELETE_FAST, depending on in which context it was emitted (whether an unbound local should raise UnboundLocalError or not).

This PR's solution is to emit DELETE_FAST_NOERROR during code generation, let the dataflow analysis potentially mark it as DELETE_FAST_NOERROR_CHECK, then replace that with LOAD_CONST(None);STORE_FAST;DELETE_FAST.

It might be fine to move add_local_null_checks to before optimize_cfg, but the small downside is that different copies of inlined blocks must share the same dataflow analysis as each other, so locking in that order of doing things isn't ideal. This affects test.test_dis.test_from_traceback_dis, where one copy of a LOAD_FAST;RETURN_VALUE is converted to LOAD_FAST_CHECK and the other isn't.

brandtbucher

I'm not sure the benefit this brings us is worth the additional complexity.

Is this still being considered for 3.13? Perhaps a future superoptimizer (#102869) could build on our already-existing liveness analysis for fast locals to achieve the same effect...

carljm · 2023-05-10T22:16:56Z

FWIW now that PEP 709 is merged, I've considered whether the same sub-scope isolation could be used for handling isolation of exception handler variables, instead of using a DELETE_FAST. I think it could, and wouldn't be too complex, and the resulting behavior would be nicer. But it would be a visible behavior change (after an except Exception as exc: clause the prior value of exc, if any, would be visible again, rather than becoming unbound again), and I'm not sure if that would rise to the level of requiring another PEP.

brandtbucher · 2023-05-10T23:06:09Z

Eh, if anything that would be better than what we currently do. It is a bit weird, though.

It personally feels like a Discuss thread more than a PEP (unless it's super controversial)...

python-cla-bot · 2025-04-18T09:44:23Z

The following commit authors need to sign the Contributor License Agreement:

36520290+sweeneyde@users.noreply.github.com

sweeneyde added 8 commits November 10, 2022 19:45

Initial implementation

a512f11

Don't modify pycore_ast

6b98c52

add DELETE_DEREF_NOERROR to fix_cell_offsets

f40d95f

update stack_effect

98df402

remove printf debugging

4c19cbc

update test_dis

6dc7ea9

fix copy/paste error

80949ec

nits

b00c114

bedevere-bot mentioned this pull request Nov 11, 2022

Avoid exc=None; del exc in bytecode where unnecessary #99360

Open

bedevere-bot added the awaiting core review label Nov 11, 2022

sweeneyde and others added 8 commits November 10, 2022 23:26

replace DELETE_FAST null check with assert

d0f1de8

GETLOCAL(oparg) = NULL

f556ea7

resolve merge conflicts

7eef58e

Add some more test cases

36edcde

Remember DELETE_FAST_CHECK. Refactor fast/slow scans to share code.

e2f7a5b

Document opcodes

e342311

fix buffer overflow from large unrelated opargs

32420e4

📜🤖 Added by blurb_it.

9658ccc

sweeneyde marked this pull request as ready for review November 15, 2022 02:28

sweeneyde requested review from brandtbucher, markshannon and iritkatriel as code owners November 15, 2022 02:28

rename to add_local_null_checks

fbf1094

iritkatriel reviewed Nov 15, 2022

View reviewed changes

use insert_instruction

9715778

iritkatriel reviewed Nov 17, 2022

View reviewed changes

sweeneyde and others added 2 commits November 21, 2022 02:16

add NOPs in compiler_nameop

165680b

Merge branch 'main' into del_noerror

88f7f19

sweeneyde marked this pull request as draft November 21, 2022 07:34

DEL_PLACEHOLDER

2d4eba5

stack_effect

c15a298

sweeneyde added 3 commits November 22, 2022 01:48

Add e=None during compiler_nameop for non-locals

0ce8a92

update doc

2ab4638

break statement

66edeb9

brandtbucher reviewed May 10, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-99360: Avoid `exc=None; del exc` in bytecode where unnecessary #99361

gh-99360: Avoid `exc=None; del exc` in bytecode where unnecessary #99361

sweeneyde commented Nov 11, 2022 •

edited

Loading

iritkatriel Nov 15, 2022

sweeneyde Nov 15, 2022

iritkatriel Nov 17, 2022

sweeneyde Nov 17, 2022

iritkatriel Nov 17, 2022

markshannon commented Nov 21, 2022

sweeneyde commented Nov 22, 2022

brandtbucher left a comment

carljm commented May 10, 2023

brandtbucher commented May 10, 2023 •

edited

Loading

python-cla-bot bot commented Apr 18, 2025

gh-99360: Avoid exc=None; del exc in bytecode where unnecessary #99361

Are you sure you want to change the base?

gh-99360: Avoid exc=None; del exc in bytecode where unnecessary #99361

Conversation

sweeneyde commented Nov 11, 2022 • edited Loading

iritkatriel Nov 15, 2022

Choose a reason for hiding this comment

sweeneyde Nov 15, 2022

Choose a reason for hiding this comment

iritkatriel Nov 17, 2022

Choose a reason for hiding this comment

sweeneyde Nov 17, 2022

Choose a reason for hiding this comment

iritkatriel Nov 17, 2022

Choose a reason for hiding this comment

markshannon commented Nov 21, 2022

sweeneyde commented Nov 22, 2022

brandtbucher left a comment

Choose a reason for hiding this comment

carljm commented May 10, 2023

brandtbucher commented May 10, 2023 • edited Loading

python-cla-bot bot commented Apr 18, 2025

gh-99360: Avoid `exc=None; del exc` in bytecode where unnecessary #99361

gh-99360: Avoid `exc=None; del exc` in bytecode where unnecessary #99361

sweeneyde commented Nov 11, 2022 •

edited

Loading

brandtbucher commented May 10, 2023 •

edited

Loading