gh-93356: Lay out exception handling code at end of code unit #92769
Conversation
|
Why does fall through from a cold block to a warm block need to be treated specially? Unless I'm mistaken: |
We can't change b->next of a block with a fallthrough. |
You could convert it to a jump and eliminate the fallthrough edge. |
That's the idea. But there could already be a conditional jump there, so if we don't want to have to worry about multiple jumps in the same block for the remaining parts of the assembler, it needs to be a new block. |
…gs calculated only once. don't pass compiler/assembler around as much
My first version did mark_cold/warm at the same time as mark_reachable. But I had a situation where the graph changed between that time and the time when I can do the push_cold_to_end. So now mark cold/warm happens just before push_cold_to_end. I could possibly share the code though. |
Python/compile.c
Outdated
| while(b && b->b_next) { | ||
| basicblock *next = b->b_next; | ||
| if (next->b_cold) { | ||
| if (next->b_next) { | ||
| b->b_next = next->b_next; | ||
| next->b_next = NULL; | ||
| tail->b_next = next; | ||
| tail = next; | ||
| } | ||
| } else { | ||
| b = next; | ||
| } | ||
| if(next == origtail) { | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
This took me a while to understand, but maybe that's just the nature of linked list code.
My biggest confusion was that meaningful fallthrough b_next linkages are broken and then re-established. There could maybe be an inner loop to scan for streaks of cold blocks, so that they can all be moved to the end with an assignment each to b.b_next, tail.b_next, last_of_streak.b_next, but I'm not sure if that makes the edge cases easier or harder.
There was a problem hiding this comment.
I think the new version is easier to follow. Let me know.
|
LGTM. I think the linked list change is clearer now, thanks. |
|
🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 6a454c8 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
|
Buildbots are happy. 🍾 |
Closes #93356.