Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

gh-93678: add _testinternalcapi.optimize_cfg() and test utils for compiler optimization unit tests #96007

Merged
merged 11 commits into from
Aug 24, 2022

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Aug 15, 2022

…est utils for compiler optimization unit tests
@iritkatriel iritkatriel added tests Tests in the Lib/test dir interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Aug 15, 2022
@iritkatriel iritkatriel changed the title gh-93678: added _testinternalcapi.optimize_cfg() function and test utils for compiler optimization unit tests gh-93678: added _testinternalcapi.optimize_cfg() and test utils for compiler optimization unit tests Aug 15, 2022
Python/compile.c Outdated
for (int i = 0; i < b->b_iused; i++) {
struct instr *instr = &b->b_instr[i];
struct location loc = instr->i_loc;
long long arg = instr->i_oparg;
Copy link
Member

Choose a reason for hiding this comment

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

Why 64bit, not 32bit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Two lines down, instr->i_target is a pointer, and I'm passing its address back to python as the label of the block. (It get normalised by the test harness).

Copy link
Member

Choose a reason for hiding this comment

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

If it is an address, it should be uintptr_t, and add a comment that it just an ID.

Copy link
Member Author

Choose a reason for hiding this comment

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

With uintptr_t the test fail on the Windows x86 buildbot.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed to (small) calculated block IDs, so we don't have the issue anymore.

@iritkatriel iritkatriel self-assigned this Aug 16, 2022
@iritkatriel iritkatriel changed the title gh-93678: added _testinternalcapi.optimize_cfg() and test utils for compiler optimization unit tests gh-93678: add _testinternalcapi.optimize_cfg() and test utils for compiler optimization unit tests Aug 22, 2022
@iritkatriel iritkatriel added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 23, 2022
@bedevere-bot
Copy link

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

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 23, 2022
@iritkatriel
Copy link
Member Author

The freebsd test failure seems unrelated and affects other PRs so I think this is ready.

@iritkatriel iritkatriel merged commit 420f39f into python:main Aug 24, 2022
mdboom pushed a commit to mdboom/cpython that referenced this pull request Aug 24, 2022
@iritkatriel iritkatriel deleted the optimize_func_1 branch September 29, 2022 17:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants