GH-146073: Remove redundancy in _PyJit_TryInitializeTracing#148301
GH-146073: Remove redundancy in _PyJit_TryInitializeTracing#148301markshannon wants to merge 1 commit intopython:mainfrom
_PyJit_TryInitializeTracing#148301Conversation
Fidget-Spinner
left a comment
There was a problem hiding this comment.
This will likely hurt performance as loops close more rarely.
close_loop_instr points to the loop header. this_instr points to the current jump backwards. When I implemented this, I distinctly remembering wanting to close the loops more frequently. With this change, loops will have to reach the exact same backwards jump to close the loop, which is not want we want as those are unlikely for loops with many backward jumps.
The likelihood of a loop closing via hitting the loop header is significantly higher than hitting the exact same backwards jump found.
|
When you're done making the requested changes, leave the comment: |
The jump backwards immediately precedes the the loop header, so the loop header will never be reached when tracing a loop. Even if the having two stop points does work better now, they are getting in the way of choosing when to stop tracing with fitness and exit-quality, so we should remove the extra field anyway. |
This is why we check that the jump backwards jumps to close_loop_instr or the start instruction, so another jump backwards will still be able to terminate the trace.
Do you have perf numbers for removing the second stop points? Also if this is the case, I would prefer the stop point be the loop header, not the original JUMP_BACKWARD. |
Small cleanup to assist implementing #146073