GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-87092: move CFG related code from compile.c to flowgraph.c by iritkatriel · Pull Request #103021 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Mar 24, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • 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 2 commits March 24, 2023 19:52
@iritkatriel
pythongh-87092: create new opcode_utils.h file and move macros from c…
5eea154 
…ompile.c there, so they can be shared
@iritkatriel
pythongh-87092: move CFG related code from compile.c to cfg.c
b5244da
@iritkatriel
iritkatriel requested a review from markshannon as a code owner March 24, 2023 23:17
@bedevere-bot bedevere-bot mentioned this pull request Mar 24, 2023
Split compiler into code-gen, optimizer and assembler. #87092
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Mar 24, 2023
@iritkatriel iritkatriel added skip news interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 24, 2023
iritkatriel added 3 commits March 24, 2023 23:40
@iritkatriel
remove include
abb6f26
@iritkatriel
make globals const
91a8743
@iritkatriel
update pcbuild
abaeeac
@iritkatriel
iritkatriel requested a review from a team as a code owner March 25, 2023 18:48
@iritkatriel iritkatriel added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 25, 2023
@bedevere-bot

bedevere-bot commented Mar 25, 2023

Copy link
Copy Markdown

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

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 25, 2023
iritkatriel and others added 3 commits March 25, 2023 23:46
@iritkatriel
Merge branch 'main' into cfg.c
fb15754
@iritkatriel
move a few more functions from compile.c to cfg.c
8f21775
@iritkatriel
cfg.c --> cfg_opt.c
a38cbcd
@iritkatriel iritkatriel changed the title gh-87092: move CFG related code from compile.c to cfg.c gh-87092: move CFG related code from compile.c to cfg_opt.c Mar 27, 2023
iritkatriel added 4 commits March 27, 2023 18:33
@iritkatriel
update pcbuild files
0f78101
@iritkatriel
and makefile
b273dd0
@iritkatriel
fix include
5909b64
@iritkatriel
cfg_opt --> flowgraph
6f52884
@iritkatriel iritkatriel changed the title gh-87092: move CFG related code from compile.c to cfg_opt.c gh-87092: move CFG related code from compile.c to flowgraph.c Mar 29, 2023
iritkatriel and others added 2 commits March 30, 2023 11:37
@iritkatriel
Merge remote-tracking branch 'upstream/main' into cfg.c
beb28f6
@iritkatriel
Merge branch 'main' into cfg.c
b6baa7b
markshannon
markshannon requested changes Mar 31, 2023
View reviewed changes

@markshannon 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.

A bit of duplicated code, otherwise looks fine.

I don't think _PyOpcode_RelativeJump is needed any more, as all jumps are relative.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_compile.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Include/internal/pycore_opcode_utils.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Include/internal/pycore_opcode_utils.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting core review labels Mar 31, 2023
@bedevere-bot

bedevere-bot commented Mar 31, 2023

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be put in the comfy chair!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
remove duplicated macros. rename arr_ --> array
d1f0c66
@iritkatriel

iritkatriel commented Mar 31, 2023

Copy link
Copy Markdown
Member Author

I don't think _PyOpcode_RelativeJump is needed any more, as all jumps are relative.

I agree, but I'd like to do it in a separate PR because I would need to remove usages and the diff here is large enough.
markshannon reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

iritkatriel commented Mar 31, 2023

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Mar 31, 2023
@bedevere-bot

bedevere-bot commented Mar 31, 2023

Copy link
Copy Markdown

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested a review from markshannon March 31, 2023 16:47
markshannon
markshannon approved these changes Mar 31, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Mar 31, 2023
@iritkatriel
iritkatriel merged commit 80163e1 into python:main Mar 31, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 31, 2023
@iritkatriel
iritkatriel deleted the cfg.c branch April 3, 2023 17:41
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@iritkatriel @warsaw
pythongh-87092: move CFG related code from compile.c to flowgraph.c (p…
68bbf79 
…ython#103021)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon approved these changes

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

@iritkatriel @bedevere-bot @markshannon
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.