GeezerVison

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

GeezerVison is showing github.com — [mypyc] Avoid boxing/unboxing when coercing between tuple types by JukkaL · Pull Request #14899 · python/mypy

Conversation

JukkaL commented Mar 14, 2023

Copy link

Copy Markdown

Collaborator

Instead, coerce each tuple item individually. This makes some coercions between tuple types much faster, primarily because there is less (or no) allocation and deallocation going on.

This speeds up the raytrace benchmark by about 7% (when using native floats).

Related to mypyc/mypyc#99.

Sorry, something went wrong.

Uh oh!

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

All reactions

JukkaL added 2 commits

March 14, 2023 19:33


          [mypyc] Avoid boxing/unboxing when coercing between tuple types

1fd6093

Instead, coerce each tuple item individually. This makes some
coercions between tuple types much faster, primarily because there is
less (or no) allocation and deallocation going on.

This speeds up the raytrace benchmark by about 7% (when using native
floats).

Related to mypyc/mypyc#99.


          Only run test on 64 bit archictecture

72d9a9c

ichard26 approved these changes

Mar 19, 2023

View reviewed changes

Comment thread

mypyc/test-data/irbuild-i64.test

		r2 :: tuple[int64, int64]
		r1 :: tuple[int64, int64]
		L0:
		r0 = (2, 4)

ichard26 Mar 19, 2023

Copy link

Copy Markdown

Collaborator

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.

It's a little unfortunate that this remains in the IR, but it's nothing DCE can't fix if we ever implement it.

tuple_T288 CPyDef_g(void) {
    tuple_T2II cpy_r_r0;
    tuple_T288 cpy_r_r1;
CPyL0: ;
    cpy_r_r0.f0 = 2;
    cpy_r_r0.f1 = 4;
    CPyTagged_INCREF(cpy_r_r0.f0);
    CPyTagged_INCREF(cpy_r_r0.f1);
    CPyTagged_DECREF(cpy_r_r0.f0);
    CPyTagged_DECREF(cpy_r_r0.f1);
    cpy_r_r1.f0 = 1;
    cpy_r_r1.f1 = 2;
    return cpy_r_r1;
}

Sorry, something went wrong.

Uh oh!

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

All reactions

JukkaL Mar 20, 2023

Copy link

Copy Markdown

Collaborator 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.

The C compiler may optimize some of these away. Also, optimizing these shouldn't be too difficult in mypyc. Added mypyc/mypyc#983 to track this.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

mypyc/irbuild/ll_builder.py

Comment on lines +369 to +370

		# We can't reuse register values, since they can be modified.
		if not isinstance(item, Register):

ichard26 Mar 19, 2023

Copy link

Copy Markdown

Collaborator

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.

This is not a blocking comment, moreso for my own learning. What situations exist where a register is modified during the coerce operation?

Sorry, something went wrong.

Uh oh!

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

All reactions

JukkaL Mar 20, 2023

Copy link

Copy Markdown

Collaborator 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.

This is about code like this:

    t = (n, 0)
    n += 1
    t2: tuple[float, float] = t

We can't use n to refer to the first item of t when constructing t2, since n was incremented on line 2.

Sorry, something went wrong.

Uh oh!

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

All reactions

ichard26 Mar 23, 2023

Copy link

Copy Markdown

Collaborator

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.

Ah right. I should've thought of that, painfully obvious now you show it. Thanks!

Sorry, something went wrong.

Uh oh!

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

All reactions

JukkaL mentioned this pull request

Mar 20, 2023

Delete unused/dead values mypyc/mypyc#983

Open

JukkaL merged commit 1a8ea61 into master

Mar 20, 2023

JukkaL deleted the native-float-4-tuple branch

March 20, 2023 13:55

ichard26 mentioned this pull request

Jul 20, 2023

GCC on Linux fails to compile tuple assignment with somewhat complex type annotation with "array subscript 1 is above array bounds" mypyc/mypyc#885

Closed

Reviewers

1 more reviewer

ichard26

ichard26 approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

2 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 01402520 — 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.