GH-113853: Guarantee forward progress in executors#113854
Conversation
brandtbucher
left a comment
brandtbucher
left a comment
There was a problem hiding this comment.
Please also test this locally with tier 2 enabled, since we don't have CI.
|
When you're done making the requested changes, leave the comment: |
I did, and it's fine. |
|
I have made the requested changes; please review again. Thanks for the reviews. I missed a case that the old code handled: if we start at backwards jump to label I've fixed those issues, but the PR will need to be reviewed again. |
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
LGTM, just a suggestion to add another assert.
| instr += 2 - (int32_t)oparg; | ||
| if (opcode == JUMP_BACKWARD || opcode == JUMP_BACKWARD_NO_INTERRUPT) { | ||
| instr += 1 + _PyOpcode_Caches[opcode] - (int32_t)oparg; | ||
| initial_instr = instr; |
There was a problem hiding this comment.
Interesting. This will affect some of the debug output, notably the "Created a trace for ..." message below at line 676 will mention a different byte offset than the "Optimizing ..." above at line 405. I honestly like what you set it to here better, and debug level 4 is excessively verbose, so I don't care, but some bit of me will miss the correspondence. :-)
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Whoops, no, you need to re-run make regen-cases!
3 participants
Guarantees forward progress in executors by de-specializing the first tier-1 instruction before converting to uops.
Also handles loops a little differently, by comparing the next instruction with the first to find a loop.
Any backwards jump except the very first instruction terminates the trace.