gh-146058: Fix _GUARD_CODE_VERSION_*

[Fidget-Spinner]

• Issue: _GUARD_CODE_VERSION can exit to the wrong location #146058

[Fidget-Spinner]

@markshannon

[Fidget-Spinner]

PYTHON_JIT_STRESS=1 ./python -m test test_dataclasses passes on my system.

[markshannon]

Thanks for getting to this so quickly.

I would rather we do the fix now, and then add any new optimizations in a later PR.

I think it is OK to match the optimizations already done for the equivalent _GUARD_IP uops in this PR, but no more.

[bedevere-app]

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

[Fidget-Spinner]

I have made the requested changes; please review again.

[bedevere-app]

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

[markshannon]

I have made the requested changes; please review again.

You still have the new optimizations in, which I think are wrong.

[Fidget-Spinner]

I have made the requested changes; please review again.

You still have the new optimizations in, which I think are wrong.

Ok will remove them.

[bedevere-app]

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

[markshannon]

That looks good thanks.

Regarding the optimizations, we should add the symbol of the function to the abstract frame, and use that to optimize _GUARD_CODE_VERSION__PUSH_FRAME and _GUARD_IP__PUSH_FRAME

The other _GUARD_CODE_VERSION_ variants can be optimized exactly the same as the existing _GUARD_IP_ variants.

[markshannon]

It is also possible to use a code watcher to strengthen the IP guard to eliminate the code guard.
If we are at the right IP and the code object has not been deleted, then this must be the right code object.

[Fidget-Spinner]

That looks good thanks.

Regarding the optimizations, we should add the symbol of the function to the abstract frame, and use that to optimize _GUARD_CODE_VERSION__PUSH_FRAME and _GUARD_IP__PUSH_FRAME

The other _GUARD_CODE_VERSION_ variants can be optimized exactly the same as the existing _GUARD_IP_ variants.

I'm not too clear how this works, so please bear with me, I was under the impression that if the symbol points to a valid function and has a valid version it should be safe?

[markshannon]

I was under the impression that if the symbol points to a valid function and has a valid version it should be safe?

Yes, that's right.
But, the function and code objects currently stored in the abstract frame, point to the objects seen during tracing, telling us nothing about the future objects, which is why we need to use symbols.