gh-104635: Eliminate redundant STORE_FAST instructions in the compiler

[corona10]

• Issue: dead store elimination in the compiler #104635

import pyperf

runner = pyperf.Runner()
runner.timeit(name="bench dead_store",
              stmt="""
_, _, _, a = 1, 2, 3, 4
""")


Mean +- std dev: [base] 10.8 ns +- 0.2 ns -> [super] 10.1 ns +- 0.1 ns: 1.07x faster

[carljm]

This seems worth doing if the reported 1% win from pyperformance is real. We can also do a bit better here by allowing this optimization to help apply_static_swaps.

I'm not sure if there are reasons to avoid new super-instructions? Would like feedback from @markshannon on that.

[carljm]

This looks reasonable to me! Seems ok to do the swap thing either in this PR or separately. Interested in any feedback from @iritkatriel / @markshannon / @brandtbucher.

[carljm]

We might also want to expand this so it can look more than one instruction ahead for another write to the same location, skipping over an allowlist of instructions that we know don't read that same local, don't execute arbitrary code, and can't raise an exception. E.g. LOAD_FAST of a different local, POP_TOP, NOP, probably others.

[corona10]

@mdboom Hi, Can we run the pyperformance benchmark for this PR?

[corona10]

We might also want to expand this so it can look more than one instruction ahead for another write to the same location, skipping over an allowlist of instructions that we know don't read that same local, don't execute arbitrary code, and can't raise an exception.

Yeah, great idea.
Currently, we can not optimize the following case.

_, _, a, _ = foo

But if we expand the optimization, we can do it.
Do we have to expand the optimization in this PR or handle it on a separate PR?

[carljm]

Looks good to me, presuming pyperformance comes back looking neutral or better.

Do we have to expand the optimization in this PR or handle it on a separate PR?

Either way seems fine to me.

[carljm]

This is a cool benefit I didn't realize we'd also get! LOAD_*/POP_TOP pairs can be eliminated entirely.

[brandtbucher]

Ideally this would just become LOAD_CONST NOP NOP STORE_FAST though, right?

[carljm]

Yes; I think that's an orthogonal improvement to the LOAD/POP elimination optimization, allowing it to ignore intervening NOP. Or maybe it has to do rather with making it multi-pass? Or both? I haven't checked.

[corona10]

Maybe we can run the loop of optimization until there are no opcode changes.
(I didn't test yet)

[corona10]

@carljm

Looks good to me, presuming pyperformance comes back looking neutral or better.

I remeasured the benchmark with my bare metal machine,
As you expected, it shows 1.0x faster (neutral) with latest pyperformance 1.0.8.

I would like to merge this PR if possible, but it looks like we have to wait for Mark's task
And if the runtime super instruction is removed, we should re-consider adding POP_TOP__POP_TOP and POP_TOP__STORE_FAST.

The best thing would be to improve this PR with more optimization without adding super instruction or new single opcode concatenated by multiple opcodes.

Do you have any ideas or suggestions?

[carljm]

Do you have any ideas or suggestions?

I think we should wait for #105230 to land, given the motivations outlined in #105229. Then we should re-measure the performance impact of this. I think it would be good to do a stats run of pyperformance with this change and see how often POP_TOP is followed by another POP_TOP or by STORE_FAST, in order to help inform whether we should add POP_TOP_POP_TOP and POP_TOP_STORE_FAST superinstructions (in the compiler).

[corona10]

I think it would be good to do a stats run of pyperformance with this change and see how often POP_TOP is followed by another POP_TOP or by STORE_FAST,

When I ran the pyperformance, they showed the very low ratio of executions.

• POP_TOP__POP_TOP: 4,846,500 counts == 0.0 %
• POP_TOP__STORE_FAST: 19,918,740 counts == 0.0%
  https://gist.github.com/corona10/810255492523f2fa802396ca1deac73f

[corona10]

Replaced by #105320