Skip to content

Commit 6ed6cee

Browse files
committed
Address review comments
1 parent 61e9a43 commit 6ed6cee

File tree

6 files changed

+193
-181
lines changed

6 files changed

+193
-181
lines changed

Include/internal/pycore_opcode_metadata.h

Lines changed: 167 additions & 167 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/optimizer.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ translate_bytecode_to_trace(
684684
// Add one to account for the actual opcode/oparg pair:
685685
int offset = expansion->uops[i].offset + 1;
686686
switch (expansion->uops[i].size) {
687-
case OPARG_FULL:
687+
case OPARG_SIMPLE:
688688
assert(opcode != JUMP_BACKWARD_NO_INTERRUPT && opcode != JUMP_BACKWARD);
689689
break;
690690
case OPARG_CACHE_1:

Python/optimizer_bytecodes.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,14 @@ dummy_func(void) {
492492
value = sym_new_const(ctx, ptr);
493493
}
494494

495+
op(_POP_TOP_LOAD_CONST_INLINE, (ptr/4, pop -- value)) {
496+
value = sym_new_const(ctx, ptr);
497+
}
498+
499+
op(_POP_TOP_LOAD_CONST_INLINE_BORROW, (ptr/4, pop -- value)) {
500+
value = sym_new_const(ctx, ptr);
501+
}
502+
495503
op(_COPY, (bottom, unused[oparg-1] -- bottom, unused[oparg-1], top)) {
496504
assert(oparg > 0);
497505
top = bottom;
@@ -521,7 +529,6 @@ dummy_func(void) {
521529
if (watched_mutations < _Py_MAX_ALLOWED_GLOBALS_MODIFICATIONS) {
522530
PyDict_Watch(GLOBALS_WATCHER_ID, dict);
523531
_Py_BloomFilter_Add(dependencies, dict);
524-
PyObject *dict = mod->md_dict;
525532
PyObject *res = convert_global_to_const(this_instr, dict, true);
526533
attr = sym_new_const(ctx, res);
527534
}

Python/optimizer_cases.c.h

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Tools/cases_generator/analyzer.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,11 +417,13 @@ def analyze_caches(inputs: list[parser.InputEffect]) -> list[CacheEntry]:
417417
i for i in inputs if isinstance(i, parser.CacheEffect)
418418
]
419419
if caches:
420-
for cache in (caches[0], caches[-1]):
420+
# Middle entries are allowed to be unused. Check first and last caches.
421+
for index in (0, -1):
422+
cache = caches[index]
421423
if cache.name == "unused":
422-
raise analysis_error(
423-
"First cache entry in op is unused. Move to enclosing macro.", cache.tokens[0]
424-
)
424+
position = "First" if index == 0 else "Last"
425+
msg = f"{position} cache entry in op is unused. Move to enclosing macro."
426+
raise analysis_error(msg, cache.tokens[0])
425427
return [CacheEntry(i.name, int(i.size)) for i in caches]
426428

427429

Tools/cases_generator/opcode_metadata_generator.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
# Constants used instead of size for macro expansions.
2727
# Note: 1, 2, 4 must match actual cache entry sizes.
2828
OPARG_KINDS = {
29-
"OPARG_FULL": 0,
29+
"OPARG_SIMPLE": 0,
3030
"OPARG_CACHE_1": 1,
3131
"OPARG_CACHE_2": 2,
3232
"OPARG_CACHE_4": 4,
@@ -343,18 +343,20 @@ def generate_expansion_table(analysis: Analysis, out: CWriter) -> None:
343343
for part in inst.parts:
344344
size = part.size
345345
if isinstance(part, Uop):
346-
fmt = "0"
346+
# Skip specializations
347+
if "specializing" in part.annotations:
348+
continue
349+
# Add the primary expansion.
350+
fmt = "OPARG_SIMPLE"
347351
if part.name == "_SAVE_RETURN_OFFSET":
348352
fmt = "OPARG_SAVE_RETURN_OFFSET"
349353
elif part.caches:
350354
fmt = str(part.caches[0].size)
351-
# Skip specializations
352-
if "specializing" in part.annotations:
353-
continue
354355
if "replaced" in part.annotations:
355356
fmt = "OPARG_REPLACED"
356357
expansions.append((part.name, fmt, offset))
357358
if len(part.caches) > 1:
359+
# Add expansion for the second operand
358360
internal_offset = 0
359361
for cache in part.caches[:-1]:
360362
internal_offset += cache.size

0 commit comments

Comments
 (0)