Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
gh-87092: compiler's CFG construction moved to after codegen stage#102320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…r codegen and cfg_builder
bedevere-bot commentedFeb 28, 2023
🤖 New build scheduled with the buildbot fleet by@iritkatriel for commitb8f4acf 🤖 If you want to schedule another build, you need to add the🔨 test-with-buildbots label again. |
bedevere-bot commentedMar 1, 2023
🤖 New build scheduled with the buildbot fleet by@iritkatriel for commite5653ee 🤖 If you want to schedule another build, you need to add the🔨 test-with-buildbots label again. |
bedevere-bot commentedMar 2, 2023
🤖 New build scheduled with the buildbot fleet by@iritkatriel for commit788da74 🤖 If you want to schedule another build, you need to add the🔨 test-with-buildbots label again. |
The test failures are a couple of timeouts plus a couple of errors that are also showing up on other branches. |
Do we really want another instruction struct and another instruction sequence struct? typedefstructInstructionSequenceInstructionSequence;typedefstruct {InstructionSequenceinstructions;}InstructionList;typedefstruct {InstructionSequenceinstructions;}BasicBlock; The reason I'd like a single InstructionSequence, is that we can stick an object header in front of it and access it from Python. |
We can have an InstructionSequence type which is reused in basic blocks, but basic blocks have a lot more fields that we don't need for codegen, and they don't need a label map because they should have pointers to the target blocks. |
That makes sense. |
I'm not sure. The instruction in a basicblock is
We can replace the first 3 fields by an instruction struct (this makes sense, I only avoided it to keep the diff smaller). But we would still need the target pointers, or to create a parallel data structure in the basic block for the pointers (and keep it in sync with the instruction sequence when we insert or delete instructions inside a block). It would be simpler to write a function that translates a CFG to something that can be exposed to python. |
In any case, I don't think we should refactor the CFG data structures in the same PR. It will be another huge change. |
iritkatriel commentedMar 3, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I've created a function for resizing the doubling arrays, so this logic is not repeated 3 times now. To share the instruction type between codegen and cfg, we could do:
instead of the current:
Do you think we should do it in this PR? It would make the diff quit a lot messier. |
markshannon left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 couple of questions, otherwise looks good.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Thank you. I'll merge then. |
* main:pythongh-102493: fix normalization in PyErr_SetObject (python#102502)pythongh-87092: compiler's CFG construction moved to after codegen stage (python#102320)pythongh-95913: Consolidate build requirements changes in 3.11 WhatsNew (pythonGH-98781) Remove redundant `_ensure_future` in favor of `ensure_future` in `asyncio` (python#102398)pythongh-95913: Edit Faster CPython section in 3.11 WhatsNew (pythonGH-98429)pythongh-90110: Fix the c-analyzer Tool (python#102483)pythongh-101759: Update macOS installer SQLite 3.40.1 checksum (pythongh-102485) Remove unused import of `warnings` from `unittest.loader` (python#102479) Add gettext support to tools/extensions/c_annotations.py (python#101989)
* main:pythongh-102493: fix normalization in PyErr_SetObject (python#102502)pythongh-87092: compiler's CFG construction moved to after codegen stage (python#102320)pythongh-95913: Consolidate build requirements changes in 3.11 WhatsNew (pythonGH-98781) Remove redundant `_ensure_future` in favor of `ensure_future` in `asyncio` (python#102398)pythongh-95913: Edit Faster CPython section in 3.11 WhatsNew (pythonGH-98429)pythongh-90110: Fix the c-analyzer Tool (python#102483)pythongh-101759: Update macOS installer SQLite 3.40.1 checksum (pythongh-102485) Remove unused import of `warnings` from `unittest.loader` (python#102479) Add gettext support to tools/extensions/c_annotations.py (python#101989)
Uh oh!
There was an error while loading.Please reload this page.
After we merge this I will make a PR to move codegen to a separate file.
Skipping news - we will have one entry for the entire refactor once it's done.