gh-126835: Move constant tuple folding to CFG

[WolframAlph]

• Issue: Move const folding to the peephole optimizer #126835

[picnixz]

Small style comments. It's always better to split assertions into two when possible to know which condition failed.

[WolframAlph]

After moving folding to codegen (70ce2c8) I can see couple refleaks after running test suite in refleak hunter mode. After some debugging I narrowed down the problem. Long tuple defined directly does not report refleaks:

x= (0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30)

but if it is run via compile, we get refleaks:

compile("x = (0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30)", "", "single")

no idea yet why this is happening. I will be debugging further, but maybe someone has some insight what could be the problem?

[iritkatriel]

Does it leak with compile with other modes (exec, eval)?

[WolframAlph]

Does it leak with compile with other modes (exec, eval)?

Yes. I wonder if it could be some form of false positive.

[iritkatriel]

No idea.

[WolframAlph]

Problem found. Turns out codegen_addop_load_const internally creates new reference to the passed constant object.

[iritkatriel]

Why does it not happen at the prompt?

[WolframAlph]

No idea. Maybe refleak checking flaw, but not sure. Maybe someone who knows more about it could tell us?

Back to this PR - if we fold long tuples at codegen stage, we won't be able to fold long tuples that have constant expressions inside, for instance:

(1, 2, 1+2, 4, ... 100) # long constant tuple

will not be folded, unless we fold it in peepholer as written initially. Drawback of having optimizations that have to work together split between different stages.

[iritkatriel]

I tend to agree. So I guess you're saying we should choose the second option from #130769 (comment).

[WolframAlph]

Second options assumes that if we have constant tuple, we can skip STACK_USE_GUIDLINE limitation and emit long tuple bytecode relying on peepholer to optimize. However, with constant expressions inside (like in my previous example), at codegen stage we don't know if it is constant or not. And we cannot emit long tuple bytecode, unless we are sure it will be optimized by peepholer. I'm saying it looks like we should stick with optimizing in the peepholer.

[WolframAlph]

Second options assumes that if we have constant tuple, we can skip STACK_USE_GUIDLINE limitation and emit long tuple bytecode relying on peepholer to optimize. However, with constant expressions inside (like in my previous example), at codegen stage we don't know if it is constant or not. And we cannot emit long tuple bytecode, unless we are sure it will be optimized by peepholer. I'm saying it looks like we should stick with optimizing in the peepholer.

We could check recursively for constant expressions and emit long tuple bytecode if we know all subexpressions are constant, but I believe it would complicate code more than optimizing it in the peepholer.

[iritkatriel]

I understand. Ok, let's revert back to the peephole situation then.

[iritkatriel]

Another idea (not in this PR) would be to move all the STACK_USE_GUIDELINE stuff to the peephole. So codegen emits code without considering whether it's big or not, and later (after optimisations) we have a pass that finds big stack use and reduces. I don't know how this compares to what we have now in terms of code complexity.

[WolframAlph]

I will make a note and investigate later.

[WolframAlph]

I've reverted to the peepholer optimization and addressed previous review, please take a look.

[iritkatriel]

I think it's a bit simpler if we grab two instructions in each iteration.

[WolframAlph]

@iritkatriel can you review again when you have time?

[iritkatriel]

!buildbot refleak

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 4b11cf8 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130769%2Fmerge

The command will test the builders whose names match following regular expression: refleak

The builders matched are:

• AMD64 Fedora Rawhide Refleaks PR
• AMD64 Fedora Stable Refleaks PR
• aarch64 Fedora Rawhide NoGIL refleaks PR
• AMD64 CentOS9 NoGIL Refleaks PR
• aarch64 Fedora Stable Refleaks PR
• AMD64 CentOS9 Refleaks PR
• s390x RHEL9 Refleaks PR
• AMD64 Windows11 Refleaks PR
• PPC64LE Fedora Rawhide Refleaks PR
• ARM64 MacOS M1 Refleaks NoGIL PR
• s390x RHEL8 Refleaks PR
• AMD64 Fedora Rawhide NoGIL refleaks PR
• aarch64 RHEL8 Refleaks PR
• PPC64LE Fedora Rawhide NoGIL refleaks PR
• aarch64 CentOS9 Refleaks PR
• aarch64 Fedora Rawhide Refleaks PR
• AMD64 FreeBSD Refleaks PR
• PPC64LE RHEL8 Refleaks PR
• PPC64LE Fedora Stable Refleaks PR
• PPC64LE CentOS9 Refleaks PR
• AMD64 RHEL8 Refleaks PR