From a4b19243f1cc75e26f7b100f06e590b3b28523d3 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 27 Apr 2023 17:14:51 +0100 Subject: [PATCH 1/5] gh-87092: assembler uses instruction sequence instead of CFG --- Include/internal/pycore_compile.h | 8 +++ Include/internal/pycore_flowgraph.h | 3 +- Python/assemble.c | 91 ++++++++++++++--------------- Python/compile.c | 33 +++++++---- Python/flowgraph.c | 20 +++---- 5 files changed, 83 insertions(+), 72 deletions(-) diff --git a/Include/internal/pycore_compile.h b/Include/internal/pycore_compile.h index f85240c48a89b0..fe6e7f52bab356 100644 --- a/Include/internal/pycore_compile.h +++ b/Include/internal/pycore_compile.h @@ -33,11 +33,17 @@ extern int _PyAST_Optimize( struct _arena *arena, _PyASTOptimizeState *state); +typedef struct { + int h_offset; + int h_startdepth; + int h_preserve_lasti; +} _PyCompilerExceptHandlerInfo; typedef struct { int i_opcode; int i_oparg; _PyCompilerSrcLocation i_loc; + _PyCompilerExceptHandlerInfo i_except_handler_info; } _PyCompilerInstruction; typedef struct { @@ -82,6 +88,8 @@ int _PyCompile_EnsureArrayLargeEnough( int _PyCompile_ConstCacheMergeOne(PyObject *const_cache, PyObject **obj); +int _PyCompile_InstrSize(int opcode, int oparg); + /* Access compiler internals for unit testing */ PyAPI_FUNC(PyObject*) _PyCompile_CodeGen( diff --git a/Include/internal/pycore_flowgraph.h b/Include/internal/pycore_flowgraph.h index f470dad3aaa459..af8cd7eb67abd0 100644 --- a/Include/internal/pycore_flowgraph.h +++ b/Include/internal/pycore_flowgraph.h @@ -97,7 +97,6 @@ int _PyCfg_OptimizeCodeUnit(_PyCfgBuilder *g, PyObject *consts, PyObject *const_ int _PyCfg_Stackdepth(_PyCfgBasicblock *entryblock, int code_flags); void _PyCfg_ConvertExceptionHandlersToNops(_PyCfgBasicblock *entryblock); int _PyCfg_ResolveJumps(_PyCfgBuilder *g); -int _PyCfg_InstrSize(_PyCfgInstruction *instruction); static inline int @@ -113,7 +112,7 @@ basicblock_nofallthrough(const _PyCfgBasicblock *b) { PyCodeObject * _PyAssemble_MakeCodeObject(_PyCompile_CodeUnitMetadata *u, PyObject *const_cache, - PyObject *consts, int maxdepth, _PyCfgBasicblock *entryblock, + PyObject *consts, int maxdepth, _PyCompile_InstructionSequence *instrs, int nlocalsplus, int code_flags, PyObject *filename); #ifdef __cplusplus diff --git a/Python/assemble.c b/Python/assemble.c index e5a361b230cf1c..cd36f1eea16aa9 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -22,8 +22,8 @@ } typedef _PyCompilerSrcLocation location; -typedef _PyCfgInstruction cfg_instr; -typedef _PyCfgBasicblock basicblock; +typedef _PyCompilerInstruction instruction; +typedef _PyCompile_InstructionSequence instr_sequence; static inline bool same_location(location a, location b) @@ -117,7 +117,8 @@ assemble_emit_exception_table_item(struct assembler *a, int value, int msb) #define MAX_SIZE_OF_ENTRY 20 static int -assemble_emit_exception_table_entry(struct assembler *a, int start, int end, basicblock *handler) +assemble_emit_exception_table_entry(struct assembler *a, int start, int end, + _PyCompilerExceptHandlerInfo *handler) { Py_ssize_t len = PyBytes_GET_SIZE(a->a_except_table); if (a->a_except_table_off + MAX_SIZE_OF_ENTRY >= len) { @@ -125,13 +126,13 @@ assemble_emit_exception_table_entry(struct assembler *a, int start, int end, bas } int size = end-start; assert(end > start); - int target = handler->b_offset; - int depth = handler->b_startdepth - 1; - if (handler->b_preserve_lasti) { + int target = handler->h_offset; + int depth = handler->h_startdepth - 1; + if (handler->h_preserve_lasti) { depth -= 1; } assert(depth >= 0); - int depth_lasti = (depth<<1) | handler->b_preserve_lasti; + int depth_lasti = (depth<<1) | handler->h_preserve_lasti; assemble_emit_exception_table_item(a, start, (1<<7)); assemble_emit_exception_table_item(a, size, 0); assemble_emit_exception_table_item(a, target, 0); @@ -140,29 +141,26 @@ assemble_emit_exception_table_entry(struct assembler *a, int start, int end, bas } static int -assemble_exception_table(struct assembler *a, basicblock *entryblock) +assemble_exception_table(struct assembler *a, instr_sequence *instrs) { - basicblock *b; int ioffset = 0; - basicblock *handler = NULL; + _PyCompilerExceptHandlerInfo handler; + handler.h_offset = -1; int start = -1; - for (b = entryblock; b != NULL; b = b->b_next) { - ioffset = b->b_offset; - for (int i = 0; i < b->b_iused; i++) { - cfg_instr *instr = &b->b_instr[i]; - if (instr->i_except != handler) { - if (handler != NULL) { - RETURN_IF_ERROR( - assemble_emit_exception_table_entry(a, start, ioffset, handler)); - } - start = ioffset; - handler = instr->i_except; + for (int i = 0; i < instrs->s_used; i++) { + instruction *instr = &instrs->s_instrs[i]; + if (instr->i_except_handler_info.h_offset != handler.h_offset) { + if (handler.h_offset >= 0) { + RETURN_IF_ERROR( + assemble_emit_exception_table_entry(a, start, ioffset, &handler)); } - ioffset += _PyCfg_InstrSize(instr); + start = ioffset; + handler = instr->i_except_handler_info; } + ioffset += _PyCompile_InstrSize(instr->i_opcode, instr->i_oparg); } - if (handler != NULL) { - RETURN_IF_ERROR(assemble_emit_exception_table_entry(a, start, ioffset, handler)); + if (handler.h_offset >= 0) { + RETURN_IF_ERROR(assemble_emit_exception_table_entry(a, start, ioffset, &handler)); } return SUCCESS; } @@ -316,31 +314,31 @@ assemble_emit_location(struct assembler* a, location loc, int isize) } static int -assemble_location_info(struct assembler *a, basicblock *entryblock, int firstlineno) +assemble_location_info(struct assembler *a, instr_sequence *instrs, + int firstlineno) { a->a_lineno = firstlineno; location loc = NO_LOCATION; int size = 0; - for (basicblock *b = entryblock; b != NULL; b = b->b_next) { - for (int j = 0; j < b->b_iused; j++) { - if (!same_location(loc, b->b_instr[j].i_loc)) { + for (int i = 0; i < instrs->s_used; i++) { + instruction *instr = &instrs->s_instrs[i]; + if (!same_location(loc, instr->i_loc)) { RETURN_IF_ERROR(assemble_emit_location(a, loc, size)); - loc = b->b_instr[j].i_loc; + loc = instr->i_loc; size = 0; - } - size += _PyCfg_InstrSize(&b->b_instr[j]); } + size += _PyCompile_InstrSize(instr->i_opcode, instr->i_oparg); } RETURN_IF_ERROR(assemble_emit_location(a, loc, size)); return SUCCESS; } static void -write_instr(_Py_CODEUNIT *codestr, cfg_instr *instruction, int ilen) +write_instr(_Py_CODEUNIT *codestr, instruction *instr, int ilen) { - int opcode = instruction->i_opcode; + int opcode = instr->i_opcode; assert(!IS_PSEUDO_OPCODE(opcode)); - int oparg = instruction->i_oparg; + int oparg = instr->i_oparg; assert(HAS_ARG(opcode) || oparg == 0); int caches = _PyOpcode_Caches[opcode]; switch (ilen - caches) { @@ -380,12 +378,12 @@ write_instr(_Py_CODEUNIT *codestr, cfg_instr *instruction, int ilen) */ static int -assemble_emit_instr(struct assembler *a, cfg_instr *i) +assemble_emit_instr(struct assembler *a, instruction *instr) { Py_ssize_t len = PyBytes_GET_SIZE(a->a_bytecode); _Py_CODEUNIT *code; - int size = _PyCfg_InstrSize(i); + int size = _PyCompile_InstrSize(instr->i_opcode, instr->i_oparg); if (a->a_offset + size >= len / (int)sizeof(_Py_CODEUNIT)) { if (len > PY_SSIZE_T_MAX / 2) { return ERROR; @@ -394,25 +392,24 @@ assemble_emit_instr(struct assembler *a, cfg_instr *i) } code = (_Py_CODEUNIT *)PyBytes_AS_STRING(a->a_bytecode) + a->a_offset; a->a_offset += size; - write_instr(code, i, size); + write_instr(code, instr, size); return SUCCESS; } static int -assemble_emit(struct assembler *a, basicblock *entryblock, int first_lineno, - PyObject *const_cache) +assemble_emit(struct assembler *a, instr_sequence *instrs, + int first_lineno, PyObject *const_cache) { RETURN_IF_ERROR(assemble_init(a, first_lineno)); - for (basicblock *b = entryblock; b != NULL; b = b->b_next) { - for (int j = 0; j < b->b_iused; j++) { - RETURN_IF_ERROR(assemble_emit_instr(a, &b->b_instr[j])); - } + for (int i = 0; i < instrs->s_used; i++) { + instruction *instr = &instrs->s_instrs[i]; + RETURN_IF_ERROR(assemble_emit_instr(a, instr)); } - RETURN_IF_ERROR(assemble_location_info(a, entryblock, a->a_lineno)); + RETURN_IF_ERROR(assemble_location_info(a, instrs, a->a_lineno)); - RETURN_IF_ERROR(assemble_exception_table(a, entryblock)); + RETURN_IF_ERROR(assemble_exception_table(a, instrs)); RETURN_IF_ERROR(_PyBytes_Resize(&a->a_except_table, a->a_except_table_off)); RETURN_IF_ERROR(_PyCompile_ConstCacheMergeOne(const_cache, &a->a_except_table)); @@ -586,13 +583,13 @@ makecode(_PyCompile_CodeUnitMetadata *umd, struct assembler *a, PyObject *const_ PyCodeObject * _PyAssemble_MakeCodeObject(_PyCompile_CodeUnitMetadata *umd, PyObject *const_cache, - PyObject *consts, int maxdepth, basicblock *entryblock, + PyObject *consts, int maxdepth, instr_sequence *instrs, int nlocalsplus, int code_flags, PyObject *filename) { PyCodeObject *co = NULL; struct assembler a; - int res = assemble_emit(&a, entryblock, umd->u_firstlineno, const_cache); + int res = assemble_emit(&a, instrs, umd->u_firstlineno, const_cache); if (res == SUCCESS) { co = makecode(umd, &a, const_cache, consts, maxdepth, nlocalsplus, code_flags, filename); diff --git a/Python/compile.c b/Python/compile.c index a0ad3687f586d8..096abe7ee51b7d 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -149,6 +149,17 @@ enum { COMPILER_SCOPE_COMPREHENSION, }; + +int +_PyCompile_InstrSize(int opcode, int oparg) +{ + assert(!IS_PSEUDO_OPCODE(opcode)); + assert(HAS_ARG(opcode) || oparg == 0); + int extended_args = (0xFFFFFF < oparg) + (0xFFFF < oparg) + (0xFF < oparg); + int caches = _PyOpcode_Caches[opcode]; + return extended_args + 1 + caches; +} + typedef _PyCompilerInstruction instruction; typedef _PyCompile_InstructionSequence instr_sequence; @@ -6968,10 +6979,6 @@ optimize_and_assemble_code_unit(struct compiler_unit *u, PyObject *const_cache, goto error; } - if (cfg_to_instr_sequence(&g, &optimized_instrs) < 0) { - goto error; - } - /** Assembly **/ int nlocalsplus = prepare_localsplus(u, &g, code_flags); if (nlocalsplus < 0) { @@ -6994,11 +7001,10 @@ optimize_and_assemble_code_unit(struct compiler_unit *u, PyObject *const_cache, goto error; } - /* Can't modify the bytecode after computing jump offsets. */ co = _PyAssemble_MakeCodeObject(&u->u_metadata, const_cache, consts, - maxdepth, g.g_entryblock, nlocalsplus, + maxdepth, &optimized_instrs, nlocalsplus, code_flags, filename); error: @@ -7039,11 +7045,18 @@ cfg_to_instr_sequence(cfg_builder *g, instr_sequence *seq) RETURN_IF_ERROR(instr_sequence_use_label(seq, b->b_label.id)); for (int i = 0; i < b->b_iused; i++) { cfg_instr *instr = &b->b_instr[i]; - int arg = HAS_TARGET(instr->i_opcode) ? - instr->i_target->b_label.id : - instr->i_oparg; RETURN_IF_ERROR( - instr_sequence_addop(seq, instr->i_opcode, arg, instr->i_loc)); + instr_sequence_addop(seq, instr->i_opcode, instr->i_oparg, instr->i_loc)); + + _PyCompilerExceptHandlerInfo *hi = &seq->s_instrs[seq->s_used-1].i_except_handler_info; + if (instr->i_except != NULL) { + hi->h_offset = instr->i_except->b_offset; + hi->h_startdepth = instr->i_except->b_startdepth; + hi->h_preserve_lasti = instr->i_except->b_preserve_lasti; + } + else { + hi->h_offset = -1; + } } } return SUCCESS; diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 67cc5c5e88be10..6f83a910cab392 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -166,16 +166,10 @@ _PyBasicblock_InsertInstruction(basicblock *block, int pos, cfg_instr *instr) { return SUCCESS; } -int -_PyCfg_InstrSize(cfg_instr *instruction) +static int +instr_size(cfg_instr *instruction) { - int opcode = instruction->i_opcode; - assert(!IS_PSEUDO_OPCODE(opcode)); - int oparg = instruction->i_oparg; - assert(HAS_ARG(opcode) || oparg == 0); - int extended_args = (0xFFFFFF < oparg) + (0xFFFF < oparg) + (0xFF < oparg); - int caches = _PyOpcode_Caches[opcode]; - return extended_args + 1 + caches; + return _PyCompile_InstrSize(instruction->i_opcode, instruction->i_oparg); } static int @@ -183,7 +177,7 @@ blocksize(basicblock *b) { int size = 0; for (int i = 0; i < b->b_iused; i++) { - size += _PyCfg_InstrSize(&b->b_instr[i]); + size += instr_size(&b->b_instr[i]); } return size; } @@ -492,7 +486,7 @@ resolve_jump_offsets(basicblock *entryblock) bsize = b->b_offset; for (int i = 0; i < b->b_iused; i++) { cfg_instr *instr = &b->b_instr[i]; - int isize = _PyCfg_InstrSize(instr); + int isize = instr_size(instr); /* jump offsets are computed relative to * the instruction pointer after fetching * the jump instruction. @@ -508,7 +502,7 @@ resolve_jump_offsets(basicblock *entryblock) assert(!IS_BACKWARDS_JUMP_OPCODE(instr->i_opcode)); instr->i_oparg -= bsize; } - if (_PyCfg_InstrSize(instr) != isize) { + if (instr_size(instr) != isize) { extended_arg_recompile = 1; } } @@ -520,7 +514,7 @@ resolve_jump_offsets(basicblock *entryblock) with a better solution. The issue is that in the first loop blocksize() is called - which calls _PyCfg_InstrSize() which requires i_oparg be set + which calls instr_size() which requires i_oparg be set appropriately. There is a bootstrap problem because i_oparg is calculated in the second loop above. From 5f8764251408f7949ba9855dcce9459dd01b24cf Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 27 Apr 2023 17:56:27 +0100 Subject: [PATCH 2/5] assemble.c doesn't need to include pycore_flowgraph.c --- Include/internal/pycore_compile.h | 1 + Include/internal/pycore_flowgraph.h | 1 - Python/assemble.c | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_compile.h b/Include/internal/pycore_compile.h index fe6e7f52bab356..7b484e640848a8 100644 --- a/Include/internal/pycore_compile.h +++ b/Include/internal/pycore_compile.h @@ -19,6 +19,7 @@ PyAPI_FUNC(PyCodeObject*) _PyAST_Compile( int optimize, struct _arena *arena); +static const _PyCompilerSrcLocation NO_LOCATION = {-1, -1, -1, -1}; typedef struct { int optimize; diff --git a/Include/internal/pycore_flowgraph.h b/Include/internal/pycore_flowgraph.h index af8cd7eb67abd0..883334f4b182eb 100644 --- a/Include/internal/pycore_flowgraph.h +++ b/Include/internal/pycore_flowgraph.h @@ -11,7 +11,6 @@ extern "C" { #include "pycore_opcode_utils.h" #include "pycore_compile.h" -static const _PyCompilerSrcLocation NO_LOCATION = {-1, -1, -1, -1}; typedef struct { int i_opcode; diff --git a/Python/assemble.c b/Python/assemble.c index cd36f1eea16aa9..d315147d620fb9 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -1,10 +1,10 @@ #include #include "Python.h" -#include "pycore_flowgraph.h" +#include "pycore_code.h" // write_location_entry_start() #include "pycore_compile.h" +#include "pycore_opcode.h" // _PyOpcode_Caches[] and opcode category macros #include "pycore_pymem.h" // _PyMem_IsPtrFreed() -#include "pycore_code.h" // write_location_entry_start() #define DEFAULT_CODE_SIZE 128 From 74a7b4013836c31857584bd0e0f75f9494e55e4d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 27 Apr 2023 18:21:25 +0100 Subject: [PATCH 3/5] rename _PyCompilerExceptHandlerInfo --> _PyCompile_ExceptHandlerInfo --- Include/internal/pycore_compile.h | 4 ++-- Python/assemble.c | 4 ++-- Python/compile.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_compile.h b/Include/internal/pycore_compile.h index 7b484e640848a8..d9f9044919501e 100644 --- a/Include/internal/pycore_compile.h +++ b/Include/internal/pycore_compile.h @@ -38,13 +38,13 @@ typedef struct { int h_offset; int h_startdepth; int h_preserve_lasti; -} _PyCompilerExceptHandlerInfo; +} _PyCompile_ExceptHandlerInfo; typedef struct { int i_opcode; int i_oparg; _PyCompilerSrcLocation i_loc; - _PyCompilerExceptHandlerInfo i_except_handler_info; + _PyCompile_ExceptHandlerInfo i_except_handler_info; } _PyCompilerInstruction; typedef struct { diff --git a/Python/assemble.c b/Python/assemble.c index d315147d620fb9..b9580559f893a5 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -118,7 +118,7 @@ assemble_emit_exception_table_item(struct assembler *a, int value, int msb) static int assemble_emit_exception_table_entry(struct assembler *a, int start, int end, - _PyCompilerExceptHandlerInfo *handler) + _PyCompile_ExceptHandlerInfo *handler) { Py_ssize_t len = PyBytes_GET_SIZE(a->a_except_table); if (a->a_except_table_off + MAX_SIZE_OF_ENTRY >= len) { @@ -144,7 +144,7 @@ static int assemble_exception_table(struct assembler *a, instr_sequence *instrs) { int ioffset = 0; - _PyCompilerExceptHandlerInfo handler; + _PyCompile_ExceptHandlerInfo handler; handler.h_offset = -1; int start = -1; for (int i = 0; i < instrs->s_used; i++) { diff --git a/Python/compile.c b/Python/compile.c index 096abe7ee51b7d..c33ee7b8dc95f4 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7048,7 +7048,7 @@ cfg_to_instr_sequence(cfg_builder *g, instr_sequence *seq) RETURN_IF_ERROR( instr_sequence_addop(seq, instr->i_opcode, instr->i_oparg, instr->i_loc)); - _PyCompilerExceptHandlerInfo *hi = &seq->s_instrs[seq->s_used-1].i_except_handler_info; + _PyCompile_ExceptHandlerInfo *hi = &seq->s_instrs[seq->s_used-1].i_except_handler_info; if (instr->i_except != NULL) { hi->h_offset = instr->i_except->b_offset; hi->h_startdepth = instr->i_except->b_startdepth; From 191cc32f04d863e038cde8253eed616817260cd4 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 27 Apr 2023 19:00:55 +0100 Subject: [PATCH 4/5] rename _PyCompilerInstruction --> _PyCompile_Instruction --- Include/internal/pycore_compile.h | 4 ++-- Python/assemble.c | 2 +- Python/compile.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Include/internal/pycore_compile.h b/Include/internal/pycore_compile.h index d9f9044919501e..1a032f652dddaf 100644 --- a/Include/internal/pycore_compile.h +++ b/Include/internal/pycore_compile.h @@ -45,10 +45,10 @@ typedef struct { int i_oparg; _PyCompilerSrcLocation i_loc; _PyCompile_ExceptHandlerInfo i_except_handler_info; -} _PyCompilerInstruction; +} _PyCompile_Instruction; typedef struct { - _PyCompilerInstruction *s_instrs; + _PyCompile_Instruction *s_instrs; int s_allocated; int s_used; diff --git a/Python/assemble.c b/Python/assemble.c index b9580559f893a5..369dd8dcde9b9b 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -22,7 +22,7 @@ } typedef _PyCompilerSrcLocation location; -typedef _PyCompilerInstruction instruction; +typedef _PyCompile_Instruction instruction; typedef _PyCompile_InstructionSequence instr_sequence; static inline bool diff --git a/Python/compile.c b/Python/compile.c index c33ee7b8dc95f4..403471bba285ca 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -160,7 +160,7 @@ _PyCompile_InstrSize(int opcode, int oparg) return extended_args + 1 + caches; } -typedef _PyCompilerInstruction instruction; +typedef _PyCompile_Instruction instruction; typedef _PyCompile_InstructionSequence instr_sequence; #define INITIAL_INSTR_SEQUENCE_SIZE 100 From f51084ee879101ef67370493122ad49dbf7e5c3b Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 27 Apr 2023 22:41:52 +0100 Subject: [PATCH 5/5] move comment --- Python/compile.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 403471bba285ca..e8789def867308 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -6997,12 +6997,13 @@ optimize_and_assemble_code_unit(struct compiler_unit *u, PyObject *const_cache, if (_PyCfg_ResolveJumps(&g) < 0) { goto error; } + + /* Can't modify the bytecode after computing jump offsets. */ + if (cfg_to_instr_sequence(&g, &optimized_instrs) < 0) { goto error; } - /* Can't modify the bytecode after computing jump offsets. */ - co = _PyAssemble_MakeCodeObject(&u->u_metadata, const_cache, consts, maxdepth, &optimized_instrs, nlocalsplus, code_flags, filename);