There are a few things about this family that make it unpleasant to convert.

First, the initial call stack can be either

[method, self, arg1, arg2, ...]

or

[NULL, callable, arg1, arg2, ...]

(This situation arises because theLOAD_ATTR instruction dynamically decides whether to produce one or the other.)

In the latter case we then introspectcallable and if it is a bound method, we extract theim_func andim_self fields and convert to the former format -- but if the callable is not a bound method, we leave theNULL in place.

There is a macrois_method(stack_pointer, oparg) that, despite its lofty name, really just checks whether there's aNULL at the indicated position (usingPEEK(oparg+1), it doesn't even use thestack_pointer argument). In the converted instructions I am modeling this as an input stack effect of the formthing1, thing2, args[oparg], and I replace theis_method() macro withthing1 != NULL. Everything else then gets a little awkward, because the thing we need to call in the end is eitherthing1 orthing2, and the number of arguments is eitheroparg oroparg+1. It also means that the first argument is not the start of theargs array -- it is either that or one less.

Second, most specializations don't just call the function and produce a result. Because of Python function call inlining, most end up withDISPATCH_INLINE(), and because of that we can't leave the popping of the stack to the generated code (it would just beSTACK_SHRINK(oparg+1)) -- we have to manually do this, and while we're at it we also eitherDECREF the arguments (includingself, if present), or copy them into a new frame, depending on what we're calling.

All this leaves me with the feeling that the generator isn't a good match for the complexity of this instruction family, or I am missing a trick.

Maybe I should model the input stack effect asmethod, callable, args[oparg] and do something like

if (method != NULL) {    callable = method;    args -= 1;    oparg += 1;}

so that we can always callcallable withoparg arguments starting atargs. But we still need to recall whether we did this, so we know where to push the result.

Note that you linked this with the wrong issue.

@brandtbucher,@markshannon, any suggestions? (This is not all, I just would like some feedback on how to proceed with the remaining 12-odd specializations ofCALL.)

Why not declare the interface as(null_or_func, func_or_self, arguments[oparg] -- result) and leave it at that?

We don't need the code generator to understand very bit of C code. We aren't implementing a C compiler.

TBH, I'm a little concerned that the code generator has become an end in itself, rather than being a means to an end.

TBH, I'm a little concerned that the code generator has become an end in itself, rather than being a means to an end.

Yeah, I agree. I'm not sure what we'll get out of it. Generating a tier 2 interpreter and supporting micro-ops does still seem like a worthy cause though. We should talk a bit about this on Monday.

Another thing where the code generator is getting in the way:CHECK_EVAL_BREAKER(). This may end up withgoto handle_eval_breaker and at that pointstack_pointer andnext_instr should be updated. I guess the solution is to special-case this, and when found, move the call to just beforeDISPATCH(). We do the same thing withPREDICT(). (EDIT: Yeah, I should definitely do this, it makes things much simpler for mostCALL* opcodes.)

I have made the requested changes; please review again.

I didn't literally do what you or Mark suggested; instead, I converted everything to the format (already used by many of these)(method, callable, args[oparg] -- res) with this little idiom:

int is_meth = method != NULL;int total_args = oparg;if (is_meth) {    callable = method;    args--;    total_args++;}

(Slight variations in a few cases.) I also removed most manual stack manipulation except when exiting viaDISPATCH_INLINE(). I merged the latest main, so we now have (tadaaa)NO LEGACY INSTRUCTION DEFINITIONS LEFT.

Thanks for making the requested changes!

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

Ah, I see you already ran the benchmarks in an earlier comment. Never mind!