GeezerVison

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

GeezerVison is showing github.com — gh-104909: Implement conditional stack effects for macros by gvanrossum · Pull Request #105748 · python/cpython

Conversation

gvanrossum commented Jun 13, 2023 •

edited by bedevere-bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

Currently only conditional output effects in the last sub-instruction are supported.

This is needed so we can make progress with splitting opcodes into uops with at most one cache entry or oparg -- some of the opcodes that need splitting have a conditional output effect.

Issue: Split opcodes into micro-ops #104909

Sorry, something went wrong.

Uh oh!

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

All reactions

gvanrossum added 8 commits

June 12, 2023 16:59


          Fix types and bug for pseudos

4291adf


          Example: macrofy _LOAD_GLOBAL_BUILTIN

eee069c


          Fix conditional stack output effect metadata

3bb5557


          Attempt adding conditionals to mac.stack

035d13b


          Fix macros with conditional output effect

c4b88a9

(In last op only; and no conditional input effects.)


          Revert "Example: macrofy _LOAD_GLOBAL_BUILTIN"

457e501

This reverts commit eee069c.


          Regenerate after reverting example

dec34ef


          Merge remote-tracking branch 'origin/main' into cond-effect-2

b82c4b8

gvanrossum added the skip news label

Jun 13, 2023

gvanrossum requested a review from iritkatriel

June 13, 2023 20:35

bedevere-bot added the awaiting core review label

Jun 13, 2023

bedevere-bot mentioned this pull request

Jun 13, 2023

Split opcodes into micro-ops #104909

Closed

iritkatriel reviewed

Jun 13, 2023

View reviewed changes

Comment thread

Tools/cases_generator/generate_cases.py Outdated

		src: StackEffect \| None,
		*,
		# Don't initialize from dst.cond; used for conditional outputs.
		unconditional: bool = False,

iritkatriel Jun 13, 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.

I'm finding it hard to track the directions of the different booleans here (unconditional is used for conditional outputs, and we don't initialise when it is True..).

Sorry, something went wrong.

Uh oh!

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

All reactions

gvanrossum Jun 13, 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.

That's fair. I'm reverting the sense and renaming it to init_null, with a default of True (since that's what's needed in 5 out of 6 call sites).

Sorry, something went wrong.

Uh oh!

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

All reactions


          Rename unconditional to init_null (with reverted sense)

63e498e

gvanrossum marked this pull request as draft

June 14, 2023 04:32

bedevere-bot removed the awaiting core review label

Jun 14, 2023

gvanrossum commented Jun 14, 2023

Copy link

Copy Markdown

Member Author

Further testing shows the stack adjustment is not always correct.

All reactions

Sorry, something went wrong.

Uh oh!

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


          Add test; fix stack effect bug

f9e11e0

Also remove dummy diff argument to stack_adjust().

iritkatriel reviewed

Jun 14, 2023

View reviewed changes

Comment thread

Tools/cases_generator/generate_cases.py Resolved

Uh oh!

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

gvanrossum added 3 commits

June 14, 2023 08:23


          Merge branch 'main' into cond-effect-2

958e9f1


          Make the test more thorough

fb905d7


          Better way to solve the 'init_null' problem

846a161

gvanrossum commented Jun 14, 2023

Copy link

Copy Markdown

Member Author

I'll undraft this once gh-105788 is merged (which is one of the earlier commits in this PR, but should be separated out).

All reactions

Sorry, something went wrong.

Uh oh!

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

gvanrossum marked this pull request as ready for review

June 14, 2023 17:07

bedevere-bot added the awaiting core review label

Jun 14, 2023


          Merge branch 'main' into cond-effect-2

gvanrossum enabled auto-merge (squash)

June 14, 2023 20:31

gvanrossum merged commit 4caa728 into python:main

Jun 14, 2023

bedevere-bot removed the awaiting core review label

Jun 14, 2023

gvanrossum deleted the cond-effect-2 branch

June 14, 2023 21:03

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

Jun 15, 2023


          Merge branch 'main' into modulelocals

166aca9

* main: (57 commits)
  pythongh-105831: Fix NEWS blurb from pythongh-105828 (python#105833)
  pythongh-105820: Fix tok_mode expression buffer in file & readline tokenizer (python#105828)
  pythongh-105751, test_ctypes: Remove disabled tests (python#105826)
  pythongh-105821: Use a raw f-string in test_httpservers.py (python#105822)
  pythongh-105751: Remove platform usage in test_ctypes (python#105819)
  pythongh-105751: Reenable disable test_ctypes tests (python#105818)
  pythongh-105751: Remove dead code in test_ctypes (python#105817)
  More reorganisation of the typing docs (python#105787)
  Improve docs for `typing.dataclass_transform` (python#105792)
  pythonGH-89812: Churn `pathlib.Path` test methods (python#105807)
  pythongh-105800: Issue SyntaxWarning in f-strings for invalid escape sequences (python#105801)
  pythongh-105751: Cleanup test_ctypes imports (python#105803)
  pythongh-105481: add HAS_JUMP flag to opcode metadata (python#105791)
  pythongh-105751: test_ctypes avoids the operator module (pythonGH-105797)
  pythongh-105751: test_ctypes: Remove @need_symbol decorator (pythonGH-105798)
  pythongh-104909: Implement conditional stack effects for macros (python#105748)
  pythongh-75905: Remove test_xmlrpc_net: skipped since 2017 (python#105796)
  pythongh-105481: Fix types and a bug for pseudos (python#105788)
  Update DSL docs for cases generator (python#105753)
  pythonGH-77273: Better bytecodes for f-strings (pythonGH-6132)
  ...

Reviewers

iritkatriel

iritkatriel left review comments

Uh oh!

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

Assignees

No one assigned

Labels

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

gvanrossum commented Jun 13, 2023 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iritkatriel Jun 13, 2023

Choose a reason for hiding this comment

Uh oh!

gvanrossum Jun 13, 2023

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Jun 14, 2023

Uh oh!

Uh oh!

gvanrossum commented Jun 14, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gvanrossum commented Jun 13, 2023 •

edited by bedevere-bot

Loading

Uh oh!

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