gh-107557: Setup abstract interpretation #107847

Fidget-Spinner · 2023-08-10T17:43:13Z

This is joint work by @JuliaPoo and me, supported by the NUS TEST Lab.

This implements a subset of partial evaluation (specifically, constant propagation), using a technique described for our type propagation in our Tier 2 interpreter report and abstract interpretation over uops.

The main goal of upstreaming this PR is to set up the infrastructure for optimization passes of CPython uops. With constant propagation, turning global loads or attribute loads to constants at the region formation phase will open up further optimizations in subsequent passes. This PR also makes CPython uops ready for type propagation, thus allowing wide-scale typed operations.

Features:

An automatically generated abstract interpreter from the interpreter DSL.
Abstract interpretation of uops.
~~Partitioning of values on the stack and locals into static/dynamic using the concept of "partitions of nodes".~~
~~Introduces the concept of "pure" operations, and what can be ascertained about them.~~
~~Constant propagation purely via abstract interpretation of bytecode, with no requirement for SSA or AST.~~
~~An optimization pass to remove redundant SAVE_IPs after partial evaluation.~~
~~Jump target/instruction numbering, which allows us to freely relocate jump targets/instructions, as a final pass will fix them up.~~

Example:
We have the following test case.

        def testfunc(loops):
            num = 0
            while num < loops:
                x = 0
                y = 1
                z = 2
                a = x + y + z + x + y + z + x + y + z
                num += 1
            return a

This is now essentially simplified down to:

        def testfunc(loops):
            num = 0
            while num < loops:
                x = 0
                y = 1
                z = 2
                a = 9
                num += 1
            return a

Which roughly halves the trace length.

TODO:

Use lltrace convention commonly seen in rest of the uops codebase.
Cleanup.

Issue: Tier 2 abstract interpreter for the optimizer #107557

Co-Authored-By: Guido van Rossum <[email protected]>

Co-Authored-By: Jules <[email protected]>

…tion_algo

Co-Authored-By: Jules <[email protected]>

* Fix+Refactor: Handling of root nodes in special-cased type prop * Style: Removed trailing space

…WRITE` and mis-port of `PARTITIONNODE_OVERWRITE` (#41) * Fix: Inconsistent AbstractInterpContext used in PARTITIONNODE_OVERWRITE and typo in PARTITIONNODE_OVERWRITE * Style: Removed whitespace

gvanrossum

Here are some nits for everything except optimizer_analysis.c

Lib/test/test_capi/test_misc.py

Makefile.pre.in

Python/bytecodes.c

gvanrossum · 2023-08-13T02:17:53Z

Include/internal/pycore_uops.h

+    int32_t opcode;
+    int32_t oparg;


I'm curious about this change. Are you using negative opcodes or opargs? IIUC oparg really is treated as a 32-bit unsigned int in ceval.c and bytecodes.c.

I'm using both negative ints and opargs, but only during the analysis phase. They are converted back to unsigned after the last pass.

For all intents and purposes outside of the optimizer pass, they can be treated as unsigned.

Include/internal/pycore_uops.h

Tools/cases_generator/generate_cases.py

gvanrossum · 2023-08-13T02:34:11Z

Tools/cases_generator/generate_cases.py

+                        if instr.is_viable_uop() and instr.name not in SPECIALLY_HANDLED_ABSTRACT_INSTR:
+                            self.out.emit("")
+                            with self.out.block(f"case {thing.name}:"):
+                                instr.write(self.out, tier=TIER_TWO)


Note that in gh-107760 I'm removing Instruction.write altogether.

gvanrossum · 2023-08-13T02:34:35Z

Tools/cases_generator/generate_cases.py

+    a.write_abstract_interpreter_instructions(args.abstract_interpreter_cases,
+                                              args.emit_line_directives)


Heh, I noticed this too. :-)

Tools/cases_generator/generate_cases.py

gvanrossum

Looking at optimizer_analysis.c I am wondering if that code is ready for prime time. Maybe there's a way that we can add most of the infrastructure code (that is likely relatively stable) without adding any of the code that's still under heavy development? E.g. I'm fine with the extra optimizer argument and generating a new .c.h file, and even with having a dummy optimizer_analysis.c file, but I worry that the latter will undergo many serious changes that will cause a lot of churn.

It is also by far the largest amount of code, and of a highly algorithmic nature that I won't be able to review meaningfully until you and I have sat down for a review of the algorithm. Compare this to most of the Tier 2 work so far, which is mostly just engineering -- e.g. splitting bytecodes into guard and action uops that use at most one cache entry feels more like a refactoring, and the superblock generator and interpreter are also mostly just hard work, not deep thinking.

Fidget-Spinner · 2023-08-14T03:50:31Z

Yeah I'm concerned with the changes in the analysis file as well. The other problem I also thought of is that new uops (especially calls) would need modifications to the analysis file and that requires deep understanding of the algorithm.

Would you be amenable if I guarded the optimization pass behind a define that is by default off? That way we can still experiment with things but not impede other efforts.

gvanrossum · 2023-08-14T03:55:24Z

That would totally work. Can you update the PR?

Fidget-Spinner · 2023-08-14T12:15:56Z

I've decided to use an env var instead, because it's easier for testing. With a define, we would not be able to run any tests except with a custom branch of cpython.

gvanrossum · 2023-08-14T23:24:21Z

Python/bytecodes.c

@@ -3743,13 +3743,12 @@ dummy_func(
            return frame;
        }

-        op(INSERT, (--)) {
+        op(INSERT, (stuff[oparg], top -- top, stuff[oparg])) {
            // Inserts TOS at position specified by oparg
            PyObject *tos = TOP();
            for (int i = 1; i < oparg + 1; i++) {
                stack_pointer[i] = stack_pointer[i - 1];


Hm, doesn't this repeat the first stack element over and over? Maybe memmove() would do what you want? (It is supposed to be good at overlaps, unlike memcpy().)

Woops yeah thanks for catching this! Forgot to negate the indexes.

Python/bytecodes.c

Python/executor_cases.c.h

gvanrossum · 2023-08-14T23:28:20Z

Tools/cases_generator/generate_cases.py

                    case parsing.InstDef():
                        instr = AbstractInstruction(self.instrs[thing.name].inst)
                        if instr.is_viable_uop() and instr.name not in SPECIALLY_HANDLED_ABSTRACT_INSTR:
                            self.out.emit("")
                            with self.out.block(f"case {thing.name}:"):
                                instr.write(self.out, tier=TIER_TWO)


I wonder if you even need the AbstractInstruction class. Maybe you could just call a different method on Instruction?

I was thinking of how we might need to expand on it more in the future, so a separate class might be better.

When that happens in the future you can refactor the code. Until then, I recommend less code.

As you may have noticed this code gets refactored a lot. :-) It's easy because the generator is not a public API -- all we care about is whether it generates a handful of files correctly from bytecodes.c at build time. But I worry about copying and pasting code, because that's harder to refactor.

Python/optimizer.c

Fidget-Spinner · 2023-08-15T04:40:38Z

Guido, thanks for the multiple rounds of reviews. It's a gigantic PR, and I truly appreciate your time on this!

gvanrossum

I'm still concerned that we have that 1000+ line experimental file. I think requiring you to have a branch with just the changes to that file in it makes sense until it all works reliably. (For comparison, Brandt's copy-and-patch is also still a branch.)

gvanrossum · 2023-08-15T05:01:14Z

Python/executor_cases.c.h

@@ -2736,14 +2736,14 @@

        case INSERT: {
            PyObject *top;
-            PyObject **stuff;
+            PyObject **stuff1;


Hm, the warnings are kind of annoying -- let's see if we can rid of those.

Fidget-Spinner · 2023-08-15T05:36:30Z

I'm still concerned that we have that 1000+ line experimental file. I think requiring you to have a branch with just the changes to that file in it makes sense until it all works reliably. (For comparison, Brandt's copy-and-patch is also still a branch.)

Got it. I removed the experimental optimizer part and pushed it to another branch.

Python/bytecodes.c

gvanrossum

Great. Do you think this is in the state you'd like to merge now?

Fidget-Spinner · 2023-08-15T17:40:56Z

Yes I'm happy merging this!

gvanrossum

Go for it!

Fidget-Spinner and others added 30 commits August 2, 2023 17:52

pythongh-107557: Tier 2 abstract interpreter barebones

d20fbb8

📜🤖 Added by blurb_it.

2aeea51

Copy Guido's input and output code, and fix build

1a728ab

Co-Authored-By: Guido van Rossum <[email protected]>

fix separator

17fccbc

credit Jules

a1da69d

add jules to co-authors

b458e17

Co-Authored-By: Jules <[email protected]>

add pycore_optimizer.h to headers in makefile

f81f888

fix: remove whitespace

0020320

fix make smelly

1f93072

fix: build

dac63e3

fix wrong symbol

e62e015

ignore static globals check for abstract interpreter

a7f654c

Merge remote-tracking branch 'upstream/main' into abstract_interpreter

f4040b8

merge Guido's changes

ec58145

remove unused stuff

4292767

Turn on the abstract interpreter

fdcca90

Merge remote-tracking branch 'upstream/main' into abstract_interpreter

5110fb9

Merge remote-tracking branch 'origin/abstract_interpreter' into parti…

9f443a2

…tion_algo

(leaky) data structures for constant propagation

7632ed1

(with cycles) try to fix the type prop

0d0c4c4

Co-Authored-By: Jules <[email protected]>

fix: cycles

4c8953e

Co-Authored-By: Jules <[email protected]>

cleanup

3bd36fa

Co-Authored-By: Jules <[email protected]>

Fix+Refactor: Handling of root nodes in special-cased type prop (#40)

229097f

* Fix+Refactor: Handling of root nodes in special-cased type prop * Style: Removed trailing space

partially partially evaluate

ca0fab7

rename vars

68c684f

fixx off by one

46c5777

partial eval working for real this time

b839ee4

Fix: Inconsistent AbstractInterpContext used in `PARTITIONNODE_OVER…

6ecf3d2

…WRITE` and mis-port of `PARTITIONNODE_OVERWRITE` (#41) * Fix: Inconsistent AbstractInterpContext used in PARTITIONNODE_OVERWRITE and typo in PARTITIONNODE_OVERWRITE * Style: Removed whitespace

fix test, refactor, bugfix

b6eeb25

re-compute jump offsets and targets

d5cceb9

gvanrossum reviewed Aug 14, 2023

View reviewed changes

Fidget-Spinner added 4 commits August 14, 2023 19:56

address review

3c08ebe

regen

d5f16be

and env var to block tests

1e61c49

regen again

6c24b49

fix generated files

2be404d

gvanrossum reviewed Aug 14, 2023

View reviewed changes

Address review

29e255d

gvanrossum reviewed Aug 15, 2023

View reviewed changes

Fidget-Spinner added 2 commits August 15, 2023 13:30

fix up INSERT

3c44117

remove experimental parts

b758b47

Fidget-Spinner changed the title ~~gh-107557: Limited partial evaluation via abstract interpretation~~ gh-107557: Setup abstract interpretation Aug 15, 2023

revert more changes

80c7f18

gvanrossum reviewed Aug 15, 2023

View reviewed changes

Python/bytecodes.c Outdated Show resolved Hide resolved

use memmove

6a2b204

gvanrossum reviewed Aug 15, 2023

View reviewed changes

gvanrossum approved these changes Aug 15, 2023

View reviewed changes

Fidget-Spinner enabled auto-merge (squash) August 15, 2023 18:03

Fidget-Spinner merged commit e28b0dc into python:main Aug 15, 2023

Fidget-Spinner deleted the partition_algo branch August 15, 2023 18:20

bedevere-bot added awaiting merge and removed awaiting core review awaiting merge labels Aug 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-107557: Setup abstract interpretation #107847

gh-107557: Setup abstract interpretation #107847

Fidget-Spinner commented Aug 10, 2023 •

edited

Loading

gvanrossum left a comment

gvanrossum Aug 13, 2023

Fidget-Spinner Aug 14, 2023 •

edited

Loading

gvanrossum Aug 13, 2023

gvanrossum Aug 13, 2023

gvanrossum left a comment

Fidget-Spinner commented Aug 14, 2023 •

edited

Loading

gvanrossum commented Aug 14, 2023

Fidget-Spinner commented Aug 14, 2023

gvanrossum Aug 14, 2023

Fidget-Spinner Aug 15, 2023 •

edited

Loading

gvanrossum Aug 14, 2023

Fidget-Spinner Aug 15, 2023

gvanrossum Aug 15, 2023

Fidget-Spinner commented Aug 15, 2023

gvanrossum left a comment

gvanrossum Aug 15, 2023

Fidget-Spinner commented Aug 15, 2023

gvanrossum left a comment

Fidget-Spinner commented Aug 15, 2023

gvanrossum left a comment

		a.write_abstract_interpreter_instructions(args.abstract_interpreter_cases,
		args.emit_line_directives)

gh-107557: Setup abstract interpretation #107847

gh-107557: Setup abstract interpretation #107847

Conversation

Fidget-Spinner commented Aug 10, 2023 • edited Loading

gvanrossum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fidget-Spinner Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvanrossum left a comment

Choose a reason for hiding this comment

Fidget-Spinner commented Aug 14, 2023 • edited Loading

gvanrossum commented Aug 14, 2023

Fidget-Spinner commented Aug 14, 2023

Choose a reason for hiding this comment

Fidget-Spinner Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fidget-Spinner commented Aug 15, 2023

gvanrossum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fidget-Spinner commented Aug 15, 2023

gvanrossum left a comment

Choose a reason for hiding this comment

Fidget-Spinner commented Aug 15, 2023

gvanrossum left a comment

Choose a reason for hiding this comment

Fidget-Spinner commented Aug 10, 2023 •

edited

Loading

Fidget-Spinner Aug 14, 2023 •

edited

Loading

Fidget-Spinner commented Aug 14, 2023 •

edited

Loading

Fidget-Spinner Aug 15, 2023 •

edited

Loading