8000 bpo-44626: Merge basic blocks earlier to enable better handling of exit blocks without line numbers. by markshannon · Pull Request #27138 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@markshannon
Copy link
Member
@markshannon markshannon commented Jul 14, 2021

Improve propagation of line numbers to artificial blocks.
This is an incomplete, but hopefully good enough, solution.
The proper solution requires more extensive CFG modification, which I don't think the current compiler design can support.

One day, I will implement https://bugs.python.org/issue42926 🙂

https://bugs.python.org/issue44626

@markshannon markshannon changed the title Merge basic blocks earlier to enable better handling of exit blocks without line numbers. bpo-44626: Merge basic blocks earlier to enable better handling of exit blocks without line numbers. Jul 14, 2021
}
}
break;
case FOR_ITER:
Copy link
Member

Choose a reason for hiding this comment

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

You don't want to call extend_block for the default case?

Copy link
Member Author

Choose a reason for hiding this comment

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

We've already extended the blocks. I don't think there is anything to be gained by doing it again.

}
}

for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) {
Copy link
8000
Member

Choose a reason for hiding this comment

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

I am confused on why this works. Aren't the blocks supposed to be topologically linked already?

Copy link
Member Author

Choose a reason for hiding this comment

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

We haven't computed the entry block yet, so we use b_list.

It also happens that using the backwards order gives better results.
Jumps to exits tend to be forward, so going backwards will be able to combine chains of jumps that going forwards would not.

I'm not making any claims that it is complete, it may well miss some blocks, but it should be good enough.

@pablogsal pablogsal added the needs backport to 3.10 only security fixes label Jul 14, 2021
@pablogsal pablogsal merged commit a86f7da into python:main Jul 15, 2021
@miss-islington
Copy link
Contributor

Thanks @markshannon for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

Sorry @markshannon and @pablogsal, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker a86f7dae0acf918d54086cb85e5a0b0bedeedce7 3.10

@pablogsal
Copy link
Member

@markshannon Can you do the manual backport?

markshannon added a commit to markshannon/cpython that referenced this pull request Jul 16, 2021
…it blocks without line numbers (pythonGH-27138)

(cherry picked from commit a86f7da)
pablogsal pushed a commit that referenced this pull request Jul 16, 2021
…it blocks without line numbers (GH-27138) (GH-27182)

(cherry picked from commit a86f7da)
@terryjreedy terryjreedy removed the needs backport to 3.10 only security fixes label Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

0