gh-111610: Split up tier 2 opcode into opcode and target

[Fidget-Spinner]

• Issue: Split up tier 2 opcode into opcode and jump target #111610

[gvanrossum]

This is fine, but don't you want to add the code that fills in the target and the code that uses it as well? I don't know what having just this one-line addition to the struct does to help you.

[gvanrossum]

Um, executor.c just disappeared. It got merged into ceval.c.

[gvanrossum]

Isn't this the wrong place to fix this? There's already a redefined DEOPT_IF() in ceval.c around L959.

[gvanrossum]

Okay, I resolved the other conversation, but I'm still curious about this one.

[Fidget-Spinner]

Sorry I have some changes on my end that I havent pushed yet. This fixes it.

[gvanrossum]

Suggested change

	trace[trace_length].oparg = (OPARG); \
	trace[trace_length].oparg = (OPARG); \

[gvanrossum]

Nothing here is generating the code for the target stub yet. I'm putting this PR in Draft mode for now; hope you're okay with that.

[Fidget-Spinner]

Sorry I've been caught up with school. Will resume work on this next week.

[markshannon]

I'm happy to add another field to _PyUOpInstruction but I don't want to change DEOPT_IF.

The semantics of a guard is that it resumes execution of the base instruction at the same location.
That mustn't change.
It is the optimizers job to convert these de-opts to jumps. Those jumps can use the new target field.

[Fidget-Spinner]

Basically the current flow is:

tier 2 trace --- deopt + exit trace-----> tier 1

I want to change it to

tier 2 trace ----deopt---> tier 2 trace cleanup/restoration to tier 1 state ----exit trace---> tier 1

[Fidget-Spinner]

It is the optimizers job to convert these de-opts to jumps. Those jumps can use the new target field.

That's not possible when there are deopts embedded within instructions. As not all instructions have deopts separated out, e.g. _LOBAL_GLOBAL_BUILTIN.

I think we should change the semantics of deopts for tier 2. This PR only affects deopts for tier 2 not tier 1. All tier 2 deopts need to jump once we start truly inlining and re-materializing frames. I think we should standardise them and make all deopts jump.

BTW, there was a previous version of this reply, where I said the same thing as above but in the form of asking questions. After re-reading that comment a few times, I decided the tone it was putting across could be interpreted as uncooperative (sorry if anyone felt it was like that, my intention was inquisitive). I have since deleted that comment and amended it to produce this version. I'm deeply sorry that the comment did not meet my own standards for communication.

[Fidget-Spinner]

I've decided to defer this until we get frame inlining working - it adds additional complexity without any added benefits, and is only required for when we sink frame creation into exit code.