Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interpreter generator should emit code closer to pure C #102305

Closed
markshannon opened this issue Feb 27, 2023 · 5 comments
Closed

Interpreter generator should emit code closer to pure C #102305

markshannon opened this issue Feb 27, 2023 · 5 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@markshannon
Copy link
Member

markshannon commented Feb 27, 2023

Rather than emitting macros such as DISPATCH, POKE, TARGET etc, it would be useful if the code generator emitted something closer to plain C. Some macros will still be needed for portability.

Doing so would make the overhead in dispatch explicit and expose redundancies that can be eliminated.
For example, not all instructions need to save frame->prev_instr, but all do because the assignment is hidden in a macro.

Linked PRs

@gvanrossum
Copy link
Member

gvanrossum commented Feb 28, 2023

Expanding PEEK() and POKE() is easy. I think NEXTOPARG() is also straightforward, as is JUMPBY() -- I'll add those next.

For TARGET the problem is that this is conditional on whether we use computed goto:

#if USE_COMPUTED_GOTOS
#  define TARGET(op) TARGET_##op: INSTRUCTION_START(op);
#  define DISPATCH_GOTO() goto *opcode_targets[opcode]
#else
#  define TARGET(op) case op: TARGET_##op: INSTRUCTION_START(op);
#  define DISPATCH_GOTO() goto dispatch_opcode
#endif

This references INSTRUCTION_START() which in turn is dependent on Py_STATS.

I guess we could generate something like this for TARGET(op):

#if !USE_COMPUTED_GOTO
    case op:
#endif
    TARGET_##op:
        frame->prev_instr = next_instr++;
#ifdef Py_STATS
        OPCODE_EXE_INC(op);
        if (_py_stats) _py_stats->opcode_stats[lastopcode].pair_count[op]++;
        lastopcode = op;
#endif

For DISPATCH() we could expand to something like this:

    {
        _Py_CODEUNIT word = *next_instr;
        opcode = word.op.code;
        oparg = word.op.arg;
#ifdef LLTRACE
        if (lltrace) {
            lltrace_instruction(frame, stack_pointer, next_instr);
        }
#endif
        assert(cframe.use_tracing == 0 || cframe.use_tracing == 255);
        opcode |= cframe.use_tracing OR_DTRACE_LINE;
#if USE_COMPUTED_GOTO
        goto *opcode_targets[opcode]
#else
        goto dispatch_opcode;
#endif
    }

But that's a lot of code to be repeated for each instruction (170 so far!) and I'm not sure it will enlighten the reader. Also, there currently are a few places that use DISPATCH() directly (plus a whole lot more using DISPATCH_INLINED()), and keeping the macro in sync with the code generator would be an unpleasant task for future maintainers.

@markshannon Your thoughts on TARGET and DISPATCH?

@gvanrossum
Copy link
Member

(FWIW I don't see much of a benefit in the expansion of DISPATCH(), but for TARGET() I can see the point -- in the future the generator can omit frame->prev_instr = next_instr++ if the instruction cannot produce an error. The Py_STATS code should probably be a single line calling a macro though, for flexibility.)

@gvanrossum
Copy link
Member

If we're expanding TARGET(op) we should also expand PREDICTED(op). But PREDICT(op) is too complex to bother (it's almost a full copy of DISPATCH() -- see also faster-cpython/ideas#496).

@markshannon
Copy link
Member Author

Regarding, TARGET and BRANCH, it is only the label and goto that aren't portable,
We already have DISPATCH_GOTO(), so keep that. Maybe reduce TARGET(name) to just the label:

#if USE_COMPUTED_GOTO
        #define TARGET(op) TARGET_##op:
#else
        #define TARGET(op) case op:
#endif

or to avoid confusion, add a new macro LABEL(op)?

I'd leave PREDICT() for now, as we might be removing it.

gvanrossum added a commit that referenced this issue Feb 28, 2023
* Emit straight stack_pointer[-i] instead of PEEK(i), POKE(i, ...)
* Expand JUMPBY() and NEXTOPARG(), and fix a perf bug
@gvanrossum
Copy link
Member

A thought: should we also expand some of these macros where they occur in user code? E.g. JUMPBY(i) is still used frequently. Or should we just expand these in bytecodes.c?

carljm added a commit to carljm/cpython that referenced this issue Feb 28, 2023
* main: (67 commits)
  pythongh-99108: Add missing md5/sha1 defines to Modules/Setup (python#102308)
  pythongh-100227: Move _str_replace_inf to PyInterpreterState (pythongh-102333)
  pythongh-100227: Move the dtoa State to PyInterpreterState (pythongh-102331)
  pythonGH-102305: Expand some macros in generated_cases.c.h (python#102309)
  Migrate to new PSF mailgun account (python#102284)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (in Python/) (python#102193)
  pythonGH-90744: Fix erroneous doc links in the sys module (python#101319)
  pythongh-87092: Make jump target label equal to the offset of the target in the instructions sequence (python#102093)
  pythongh-101101: Unstable C API tier (PEP 689) (pythonGH-101102)
  IDLE: Simplify DynOptionsMenu __init__code (python#101371)
  pythongh-101561: Add typing.override decorator (python#101564)
  pythongh-101825: Clarify that as_integer_ratio() output is always normalized (python#101843)
  pythongh-101773: Optimize creation of Fractions in private methods (python#101780)
  pythongh-102251: Updates to test_imp Toward Fixing Some Refleaks (pythongh-102254)
  pythongh-102296 Document that inspect.Parameter kinds support ordering (pythonGH-102297)
  pythongh-102250: Fix double-decref in COMPARE_AND_BRANCH error case (pythonGH-102287)
  pythongh-101100: Fix sphinx warnings in `types` module (python#102274)
  pythongh-91038: Change default argument value to `False` instead of `0` (python#31621)
  pythongh-101765: unicodeobject: use Py_XDECREF correctly (python#102283)
  [doc] Improve grammar/fix missing word (pythonGH-102060)
  ...
@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants