GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-87092: refactor assemble() to a number of separate functions, which do not need the compiler struct by iritkatriel · Pull Request #102562 · python/cpython

Conversation

iritkatriel commented Mar 9, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

This PR:

further breaks up the assemble() function, and extracts out of it the optimize_code_unit(), assemble_code_unit(), assemble_emit() and resolve_line_numbers() steps into separate functions.
passes less stuff around (namely, not passing the whole compiler struct but only what is needed).
unit tests (_PyCompile_OptimizeCfg ) run all of optimize_code_unit(), not just optimize_cfg(), so we can test dead code removal, unused const removal, hot/cold code arrangements, etc.
_PyCompile_CodeGen no longer constructs a cfg.

Issue: Split compiler into code-gen, optimizer and assembler. #87092

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

iritkatriel added 7 commits

March 8, 2023 15:06


          remove compiler as arg from some codegen functions

8b71935


          prepare_localsplus doesn't need the whole compiler struct

ed9c7d2


          don't pass compiler around so much in assembler stage

1232d41


          extract optimize_code_unit() out of assemble()

dbf22d1


          _PyCompile_CodeGen doesn't need to construct a cfg

db4b2b8


          refactor optimize_code_unit further


          unit tests run all of the optimization stage, not just optimize_cfg

4e7857f

iritkatriel added skip news interpreter-core (Objects, Python, Grammar, and Parser dirs) labels

Mar 9, 2023

iritkatriel requested review from brandtbucher and markshannon as code owners

March 9, 2023 18:00

bedevere-bot mentioned this pull request

Mar 9, 2023

Split compiler into code-gen, optimizer and assembler. #87092

Closed

bedevere-bot added the awaiting core review label

Mar 9, 2023

iritkatriel changed the title ~~gh-87092: refactor the optimization stage out assemble() to a separate function, which does not need the compiler struct~~ gh-87092: refactor the optimization stage of assemble() to a separate function, which does not need the compiler struct

Mar 9, 2023


          break up assemble() some more

d0166c4

iritkatriel changed the title ~~gh-87092: refactor the optimization stage of assemble() to a separate function, which does not need the compiler struct~~ gh-87092: refactor assemble() to a number of separate functions, which do not need the compiler struct

Mar 10, 2023


          remove obsolete comment

c9fc294

markshannon reviewed

Mar 10, 2023

View reviewed changes

markshannon left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. A couple of questions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/support/bytecode_helper.py Outdated

		arg = oparg.value
		else:
		arg = oparg if opcode in self.HAS_ARG else None
		arg = oparg if opcode in self.HAS_ARG else 0

markshannon Mar 10, 2023

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? And why do some of the 'NOP's below in test_peepholer.py have a None operand and some have 0?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

iritkatriel Mar 10, 2023

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that was just messy. I've tidied it up now. Thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          tidy up the no-arg representation in unit test instruction sequences

0f198b6

iritkatriel merged commit 634cb61 into python:main

Mar 13, 2023

bedevere-bot removed the awaiting core review label

Mar 13, 2023

carljm added a commit to carljm/cpython that referenced this pull request

Mar 14, 2023


          Merge branch 'main' into inlinecomp2

6c5f269

* main: (50 commits)
  pythongh-102674: Remove _specialization_stats from Lib/opcode.py (python#102685)
  pythongh-102660: Handle m_copy Specially for the sys and builtins Modules (pythongh-102661)
  pythongh-102354: change python3 to python in docs examples (python#102696)
  pythongh-81057: Add a CI Check for New Unsupported C Global Variables (pythongh-102506)
  pythonGH-94851: check unicode consistency of static strings in debug mode (python#102684)
  pythongh-100315: clarification to `__slots__` docs. (python#102621)
  pythonGH-100227: cleanup initialization of global interned dict (python#102682)
  doc: Remove a duplicate 'versionchanged' in library/asyncio-task (pythongh-102677)
  pythongh-102013: Add PyUnstable_GC_VisitObjects (python#102014)
  pythonGH-102670: Use sumprod() to simplify, speed up, and improve accuracy of statistics functions (pythonGH-102649)
  pythongh-102627: Replace address pointing toward malicious web page (python#102630)
  pythongh-98831: Use DECREF_INPUTS() more (python#102409)
  pythongh-101659: Avoid Allocation for Shared Exceptions in the _xxsubinterpreters Module (pythongh-102659)
  pythongh-101524: Fix the ChannelID tp_name (pythongh-102655)
  pythongh-102069: Fix `__weakref__` descriptor generation for custom dataclasses (python#102075)
  pythongh-98169 dataclasses.astuple support DefaultDict (python#98170)
  pythongh-102650: Remove duplicate include directives from multiple source files (python#102651)
  pythonGH-100987: Don't cache references to the names and consts array in `_PyEval_EvalFrameDefault`. (python#102640)
  pythongh-87092: refactor assemble() to a number of separate functions, which do not need the compiler struct (python#102562)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (python#102631)
  ...

iritkatriel deleted the decouple_assembler branch

March 19, 2023 11:51

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request

Mar 27, 2023


          pythongh-87092: refactor assemble() to a number of separate functions…

aaca52a

…, which do not need the compiler struct (python#102562)

warsaw pushed a commit to warsaw/cpython that referenced this pull request

Apr 11, 2023


          pythongh-87092: refactor assemble() to a number of separate functions…

4c6e7d5

…, which do not need the compiler struct (python#102562)

Reviewers

markshannon

markshannon left review comments

brandtbucher

Awaiting requested review from brandtbucher

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

interpreter-core

(Objects, Python, Grammar, and Parser dirs)

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

iritkatriel commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

markshannon Mar 10, 2023

Choose a reason for hiding this comment

Uh oh!

iritkatriel Mar 10, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

iritkatriel commented Mar 9, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.