Skip to content

Conversation

brandtbucher
Copy link
Member

@brandtbucher brandtbucher commented Feb 22, 2024

Windows will come next, since that uses a different mechanism.

Somewhere in the neighborhood of ~0-3% faster, as expected.

@brandtbucher brandtbucher added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 22, 2024
@brandtbucher brandtbucher self-assigned this Feb 22, 2024
@bedevere-app bedevere-app bot mentioned this pull request Feb 22, 2024
13 tasks
@brandtbucher brandtbucher marked this pull request as ready for review February 22, 2024 19:34
@brandtbucher brandtbucher merged commit 7259480 into python:main Feb 26, 2024
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

I minor issue with an assert, otherwise looks good.

{
unsigned char rd = get_bits(loc32[0], 0, 5);
assert(IS_AARCH64_LDR_OR_STR(loc32[1]));
unsigned char rt = get_bits(loc32[1], 0, 5);
Copy link
Member

Choose a reason for hiding this comment

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

rt and rn are only used in the assert, so I'm getting an unused variable warning.

assert(IS_AARCH64_LDR_OR_STR(loc32[1]));
unsigned char rt = get_bits(loc32[1], 0, 5);
unsigned char rn = get_bits(loc32[1], 5, 5);
assert(rd == rn && rn == rt);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
assert(rd == rn && rn == rt);
assert(rd == get_bits(loc32[1], 5, 5) && rd == get_bits(loc32[1], 0, 5));

"-fno-jump-tables",
# Position-independent code adds indirection to every load and jump:
"-fno-pic",
"-fno-plt",
Copy link
Member

Choose a reason for hiding this comment

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

Could you make sure each option has a comment saying what it does.

@bedevere-app
Copy link

bedevere-app bot commented Feb 26, 2024

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

@brandtbucher
Copy link
Member Author

Sorry, looks like I merged just before your review. I'll address these in the follow-up Windows PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage skip news topic-JIT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants