Skip to content

gh-130080: move _Py_EnsureArrayLargeEnough to a separate header so it can be used outside of the compiler #130930

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

Merged
merged 9 commits into from
Mar 13, 2025
39 changes: 39 additions & 0 deletions Include/internal/pycore_c_array.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#ifndef Py_INTERNAL_C_ARRAY_H
#define Py_INTERNAL_C_ARRAY_H

#ifdef __cplusplus
extern "C" {
#endif

#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif


/* Utility for a number of growing arrays */

typedef struct {
void *array; /* pointer to the array */
int allocated_entries; /* pointer to the capacity of the array */
size_t item_size; /* size of each element */
int initial_num_entries; /* initial allocation size */
} _Py_c_array_t;


int _Py_CArray_Init(_Py_c_array_t* array, int item_size, int initial_num_entries);
void _Py_CArray_Fini(_Py_c_array_t* array);

/* If idx is out of bounds:
* If arr->array is NULL, allocate arr->initial_num_entries slots.
* Otherwise, double its size.
*
* Return 0 if successful and -1 (with exception set) otherwise.
*/
int _Py_CArray_EnsureCapacity(_Py_c_array_t *c_array, int idx);


#ifdef __cplusplus
}
#endif

#endif /* !Py_INTERNAL_C_ARRAY_H */
8 changes: 0 additions & 8 deletions Include/internal/pycore_compile.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,6 @@ int _PyCodegen_Expression(struct _PyCompiler *c, expr_ty e);
int _PyCodegen_Body(struct _PyCompiler *c, _Py_SourceLocation loc, asdl_stmt_seq *stmts,
bool is_interactive);

/* Utility for a number of growing arrays used in the compiler */
int _PyCompile_EnsureArrayLargeEnough(
int idx,
void **array,
int *alloc,
int default_alloc,
size_t item_size);

int _PyCompile_ConstCacheMergeOne(PyObject *const_cache, PyObject **obj);

PyCodeObject *_PyCompile_OptimizeAndAssemble(struct _PyCompiler *c, int addNone);
Expand Down
58 changes: 34 additions & 24 deletions Python/codegen.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#define NEED_OPCODE_TABLES
#include "pycore_opcode_utils.h"
#undef NEED_OPCODE_TABLES
#include "pycore_c_array.h" // _Py_c_array_t
#include "pycore_compile.h"
#include "pycore_instruction_sequence.h" // _PyInstructionSequence_NewLabel()
#include "pycore_intrinsics.h"
Expand Down Expand Up @@ -100,40 +101,48 @@ static const int compare_masks[] = {
[Py_GE] = COMPARISON_GREATER_THAN | COMPARISON_EQUALS,
};

/*
* Resize the array if index is out of range.
*
* idx: the index we want to access
* arr: pointer to the array
* alloc: pointer to the capacity of the array
* default_alloc: initial number of items
* item_size: size of each item
*
*/

int
_Py_CArray_Init(_Py_c_array_t* array, int item_size, int initial_num_entries) {
memset(array, 0, sizeof(_Py_c_array_t));
array->item_size = item_size;
array->initial_num_entries = initial_num_entries;
return 0;
}

void
_Py_CArray_Fini(_Py_c_array_t* array)
{
if (array->array) {
PyMem_Free(array->array);
array->allocated_entries = 0;
}
}

int
_PyCompile_EnsureArrayLargeEnough(int idx, void **array, int *alloc,
int default_alloc, size_t item_size)
_Py_CArray_EnsureCapacity(_Py_c_array_t *c_array, int idx)
{
void *arr = *array;
void *arr = c_array->array;
int alloc = c_array->allocated_entries;
if (arr == NULL) {
int new_alloc = default_alloc;
int new_alloc = c_array->initial_num_entries;
if (idx >= new_alloc) {
new_alloc = idx + default_alloc;
new_alloc = idx + c_array->initial_num_entries;
}
arr = PyMem_Calloc(new_alloc, item_size);
arr = PyMem_Calloc(new_alloc, c_array->item_size);
if (arr == NULL) {
PyErr_NoMemory();
return ERROR;
}
*alloc = new_alloc;
alloc = new_alloc;
}
else if (idx >= *alloc) {
size_t oldsize = *alloc * item_size;
int new_alloc = *alloc << 1;
else if (idx >= alloc) {
size_t oldsize = alloc * c_array->item_size;
int new_alloc = alloc << 1;
if (idx >= new_alloc) {
new_alloc = idx + default_alloc;
new_alloc = idx + c_array->initial_num_entries;
}
size_t newsize = new_alloc * item_size;
size_t newsize = new_alloc * c_array->item_size;

if (oldsize > (SIZE_MAX >> 1)) {
PyErr_NoMemory();
Expand All @@ -146,12 +155,13 @@ _PyCompile_EnsureArrayLargeEnough(int idx, void **array, int *alloc,
PyErr_NoMemory();
return ERROR;
}
*alloc = new_alloc;
alloc = new_alloc;
arr = tmp;
memset((char *)arr + oldsize, 0, newsize - oldsize);
}

*array = arr;
c_array->array = arr;
c_array->allocated_entries = alloc;
return SUCCESS;
}

Expand Down
18 changes: 11 additions & 7 deletions Python/flowgraph.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "Python.h"
#include "opcode.h"
#include "pycore_c_array.h" // _Py_CArray_EnsureCapacity
#include "pycore_flowgraph.h"
#include "pycore_compile.h"
#include "pycore_intrinsics.h"
Expand Down Expand Up @@ -141,13 +142,16 @@ static int
basicblock_next_instr(basicblock *b)
{
assert(b != NULL);
RETURN_IF_ERROR(
_PyCompile_EnsureArrayLargeEnough(
b->b_iused + 1,
(void**)&b->b_instr,
&b->b_ialloc,
DEFAULT_BLOCK_SIZE,
sizeof(cfg_instr)));
_Py_c_array_t array = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to manually initialize a struct, just to pass it to a function seems clunky. The previous API was probably better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just to make the transition now. If we were defining basic block from scratch we would have put this struct field in it. But I think now that's a transformation for another PR.

.array = (void*)b->b_instr,
.allocated_entries = b->b_ialloc,
.item_size = sizeof(cfg_instr),
.initial_num_entries = DEFAULT_BLOCK_SIZE,
};

RETURN_IF_ERROR(_Py_CArray_EnsureCapacity(&array, b->b_iused + 1));
b->b_instr = array.array;
b->b_ialloc = array.allocated_entries;
return b->b_iused++;
}

Expand Down
37 changes: 24 additions & 13 deletions Python/instruction_sequence.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@

#include "Python.h"

#include "pycore_compile.h" // _PyCompile_EnsureArrayLargeEnough
#include "pycore_c_array.h" // _Py_CArray_EnsureCapacity
#include "pycore_compile.h" // _PyInstruction
#include "pycore_opcode_utils.h"
#include "pycore_opcode_metadata.h" // OPCODE_HAS_ARG, etc

Expand Down Expand Up @@ -36,12 +37,18 @@ static int
instr_sequence_next_inst(instr_sequence *seq) {
assert(seq->s_instrs != NULL || seq->s_used == 0);

RETURN_IF_ERROR(
_PyCompile_EnsureArrayLargeEnough(seq->s_used + 1,
(void**)&seq->s_instrs,
&seq->s_allocated,
INITIAL_INSTR_SEQUENCE_SIZE,
sizeof(instruction)));

_Py_c_array_t array = {
.array = (void*)seq->s_instrs,
.allocated_entries = seq->s_allocated,
.item_size = sizeof(instruction),
.initial_num_entries = INITIAL_INSTR_SEQUENCE_SIZE,
};

RETURN_IF_ERROR(_Py_CArray_EnsureCapacity(&array, seq->s_used + 1));
seq->s_instrs = array.array;
seq->s_allocated = array.allocated_entries;

assert(seq->s_allocated >= 0);
assert(seq->s_used < seq->s_allocated);
return seq->s_used++;
Expand All @@ -58,12 +65,16 @@ int
_PyInstructionSequence_UseLabel(instr_sequence *seq, int lbl)
{
int old_size = seq->s_labelmap_size;
RETURN_IF_ERROR(
_PyCompile_EnsureArrayLargeEnough(lbl,
(void**)&seq->s_labelmap,
&seq->s_labelmap_size,
INITIAL_INSTR_SEQUENCE_LABELS_MAP_SIZE,
sizeof(int)));
_Py_c_array_t array = {
.array = (void*)seq->s_labelmap,
.allocated_entries = seq->s_labelmap_size,
.item_size = sizeof(int),
.initial_num_entries = INITIAL_INSTR_SEQUENCE_LABELS_MAP_SIZE,
};

RETURN_IF_ERROR(_Py_CArray_EnsureCapacity(&array, lbl));
seq->s_labelmap = array.array;
seq->s_labelmap_size = array.allocated_entries;

for(int i = old_size; i < seq->s_labelmap_size; i++) {
seq->s_labelmap[i] = -111; /* something weird, for debugging */
Expand Down
Loading