gh-98831: Modernize CALL and family

[gvanrossum]

Note that we have a few opcodes starting with CALL_ that aren't specializations of CALL.

• Issue: Generate the interpreter #98831

[gvanrossum]

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 the LOAD_ATTR instruction dynamically decides whether to produce one or the other.)

In the latter case we then introspect callable and if it is a bound method, we extract the im_func and im_self fields and convert to the former format -- but if the callable is not a bound method, we leave the NULL in place.

There is a macro is_method(stack_pointer, oparg) that, despite its lofty name, really just checks whether there's a NULL at the indicated position (using PEEK(oparg+1), it doesn't even use the stack_pointer argument). In the converted instructions I am modeling this as an input stack effect of the form thing1, thing2, args[oparg], and I replace the is_method() macro with thing1 != NULL. Everything else then gets a little awkward, because the thing we need to call in the end is either thing1 or thing2, and the number of arguments is either oparg or oparg+1. It also means that the first argument is not the start of the args 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 with DISPATCH_INLINE(), and because of that we can't leave the popping of the stack to the generated code (it would just be STACK_SHRINK(oparg+1)) -- we have to manually do this, and while we're at it we also either DECREF the arguments (including self, 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 as method, callable, args[oparg] and do something like

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

so that we can always call callable with oparg arguments starting at args. But we still need to recall whether we did this, so we know where to push the result.

[iritkatriel]

Note that you linked this with the wrong issue.

[gvanrossum]

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

[markshannon]

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.

[gvanrossum]

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.

[gvanrossum]

Another thing where the code generator is getting in the way: CHECK_EVAL_BREAKER(). This may end up with goto handle_eval_breaker and at that point stack_pointer and next_instr should be updated. I guess the solution is to special-case this, and when found, move the call to just before DISPATCH(). We do the same thing with PREDICT(). (EDIT: Yeah, I should definitely do this, it makes things much simpler for most CALL* opcodes.)

[gvanrossum]

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 via DISPATCH_INLINE(). I merged the latest main, so we now have (tadaaa) NO LEGACY INSTRUCTION DEFINITIONS LEFT.

[bedevere-bot]

Thanks for making the requested changes!

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

[brandtbucher]

Looks good!

Has it been benchmarked for regressions? Not a huge deal if not, but it does change quite a bit of critical code in subtle ways. Your call.

[brandtbucher]

Suggested change

	is_meth = 1; // For consistenct; it's dead, though
	is_meth = 1; // For consistency; it's dead, though

[brandtbucher]

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