gh-141976: Protect against non-progressing specializations in tracing JIT

[Fidget-Spinner]

I do not have a test for this as it's too hard to test for. We require the following scenario to happen:

1. Non-progressing specialization
2. that deopts

We don't have that many non-progressing specializations to begin with, so any test is a futile fight between the internal implementation details and the JIT.

I have verified this fixes the test case in the issue. I'm not using the test case in the issue as it's dependent on how much stack space we reserve and consume per stack chunk. If we ever change the stack chunks size, the test will break.

• Issue: JIT: Assertion failure for WITHIN_STACK_BOUNDS() in optimize_uops #141976

[markshannon]

This doesn't look correct to me.

The tracing front-end should produce correct code. Working around it in the optimizer is a bad idea as it just hides a bug that we should be fixing.
And the workaround doesn't make a lot of sense to me. It looks like we are missing a guard. How does checking the backoff counter help?

[Fidget-Spinner]

This doesn't look correct to me.

The tracing front-end should produce correct code. Working around it in the optimizer is a bad idea as it just hides a bug that we should be fixing.

This is fixing a bug in the tracing front-end, not the optimizer. Please see that the function is _PyJit_translate_single_bytecode_to_trace.

We are not missing a guard.

[markshannon]

This is fixing a bug in the tracing front-end, not the optimizer.

Not what you said here 😉

Maybe the tracer needs its own file.

I'm still not convinced by this PR. It is probably right, but there a number of possible deopt/specialization possibilities and I'll like to see a full table/state machine to make sure we are handling all of them.

[Fidget-Spinner]

@markshannon

These are the following states:

1. _SPECIALIZED -> _SPECIALIZED
2. GENERIC -> _SPECIALIZED
3. _SPECIALIZED -> _GENERIC (unspecialize)
4. _SPECIALIZED -> _GENERIC (did not unspecialize)

States 1 to 3 are safe, as the actual instruction executed is the right hand side. So the trace recorder recorded the correct instruction.

State transition 4 is unsafe, as the actual instruction executed is _GENERIC, but since it did not unspecialize, it's recorded as _SPECIALIZED. This is problematic as it will cause unecessary contradictions in the optimizer.

I'm quite confident this will also speed up some benchmarks that are unstable/highly polymorphic, as it was in the original design of the tracer recorder before we yanked out the counting.

[markshannon]

Transition 4 should be safe. We just end up recording the wrong specialization, so it will deopt immediately when the trace is executed.

If it is unsafe, then there is some other bug in the trace recorder.

[markshannon]

Would we better off just abandoning the trace in this case, and hoping it will be better behaved next time we optimize?

We should add stats for this, either way.

[Fidget-Spinner]

Transition 4 should be safe. We just end up recording the wrong specialization, so it will deopt immediately when the trace is executed.

If it is unsafe, then there is some other bug in the trace recorder.

Yes it should now be safe after we fixed the optimizer.