Skip to content

Commit

Permalink
pythonGH-120024: Tidy up case generator code a bit. (pythonGH-122780)
Browse files Browse the repository at this point in the history
  • Loading branch information
markshannon authored and blhsing committed Aug 22, 2024
1 parent ed4c919 commit 357a27c
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 103 deletions.
3 changes: 0 additions & 3 deletions Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

88 changes: 63 additions & 25 deletions Tools/cases_generator/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ def infallible(self) -> bool:
return not self.error_with_pop and not self.error_without_pop



SKIP_PROPERTIES = Properties(
escapes=False,
error_with_pop=False,
Expand Down Expand Up @@ -99,7 +98,6 @@ def properties(self) -> Properties:


class Flush:

@property
def properties(self) -> Properties:
return SKIP_PROPERTIES
Expand All @@ -112,6 +110,7 @@ def name(self) -> str:
def size(self) -> int:
return 0


@dataclass
class StackItem:
name: str
Expand All @@ -133,6 +132,7 @@ def is_array(self) -> bool:
def get_size(self) -> str:
return self.size if self.size else "1"


@dataclass
class StackEffect:
inputs: list[StackItem]
Expand All @@ -150,6 +150,7 @@ class CacheEntry:
def __str__(self) -> str:
return f"{self.name}/{self.size}"


@dataclass
class Uop:
name: str
Expand All @@ -163,7 +164,7 @@ class Uop:
_size: int = -1
implicitly_created: bool = False
replicated = 0
replicates : "Uop | None" = None
replicates: "Uop | None" = None

def dump(self, indent: str) -> None:
print(
Expand Down Expand Up @@ -308,19 +309,26 @@ def override_error(
)


def convert_stack_item(item: parser.StackEffect, replace_op_arg_1: str | None) -> StackItem:
def convert_stack_item(
item: parser.StackEffect, replace_op_arg_1: str | None
) -> StackItem:
cond = item.cond
if replace_op_arg_1 and OPARG_AND_1.match(item.cond):
cond = replace_op_arg_1
return StackItem(
item.name, item.type, cond, item.size
)
return StackItem(item.name, item.type, cond, item.size)


def analyze_stack(op: parser.InstDef | parser.Pseudo, replace_op_arg_1: str | None = None) -> StackEffect:
def analyze_stack(
op: parser.InstDef | parser.Pseudo, replace_op_arg_1: str | None = None
) -> StackEffect:
inputs: list[StackItem] = [
convert_stack_item(i, replace_op_arg_1) for i in op.inputs if isinstance(i, parser.StackEffect)
convert_stack_item(i, replace_op_arg_1)
for i in op.inputs
if isinstance(i, parser.StackEffect)
]
outputs: list[StackItem] = [
convert_stack_item(i, replace_op_arg_1) for i in op.outputs
]
outputs: list[StackItem] = [convert_stack_item(i, replace_op_arg_1) for i in op.outputs]
# Mark variables with matching names at the base of the stack as "peek"
modified = False
for input, output in zip(inputs, outputs):
Expand All @@ -331,9 +339,11 @@ def analyze_stack(op: parser.InstDef | parser.Pseudo, replace_op_arg_1: str | No
if isinstance(op, parser.InstDef):
output_names = [out.name for out in outputs]
for input in inputs:
if (variable_used(op, input.name) or
variable_used(op, "DECREF_INPUTS") or
(not input.peek and input.name in output_names)):
if (
variable_used(op, input.name)
or variable_used(op, "DECREF_INPUTS")
or (not input.peek and input.name in output_names)
):
input.used = True
for output in outputs:
if variable_used(op, output.name):
Expand All @@ -359,9 +369,9 @@ def analyze_deferred_refs(node: parser.InstDef) -> dict[lexer.Token, str | None]
def find_assignment_target(idx: int) -> list[lexer.Token]:
"""Find the tokens that make up the left-hand side of an assignment"""
offset = 1
for tkn in reversed(node.block.tokens[:idx-1]):
for tkn in reversed(node.block.tokens[: idx - 1]):
if tkn.kind == "SEMI" or tkn.kind == "LBRACE" or tkn.kind == "RBRACE":
return node.block.tokens[idx-offset:idx-1]
return node.block.tokens[idx - offset : idx - 1]
offset += 1
return []

Expand All @@ -370,42 +380,54 @@ def find_assignment_target(idx: int) -> list[lexer.Token]:
if tkn.kind != "IDENTIFIER" or tkn.text != "PyStackRef_FromPyObjectNew":
continue

if idx == 0 or node.block.tokens[idx-1].kind != "EQUALS":
if idx == 0 or node.block.tokens[idx - 1].kind != "EQUALS":
raise analysis_error("Expected '=' before PyStackRef_FromPyObjectNew", tkn)

lhs = find_assignment_target(idx)
if len(lhs) == 0:
raise analysis_error("PyStackRef_FromPyObjectNew() must be assigned to an output", tkn)
raise analysis_error(
"PyStackRef_FromPyObjectNew() must be assigned to an output", tkn
)

if lhs[0].kind == "TIMES" or any(t.kind == "ARROW" or t.kind == "LBRACKET" for t in lhs[1:]):
if lhs[0].kind == "TIMES" or any(
t.kind == "ARROW" or t.kind == "LBRACKET" for t in lhs[1:]
):
# Don't handle: *ptr = ..., ptr->field = ..., or ptr[field] = ...
# Assume that they are visible to the GC.
refs[tkn] = None
continue

if len(lhs) != 1 or lhs[0].kind != "IDENTIFIER":
raise analysis_error("PyStackRef_FromPyObjectNew() must be assigned to an output", tkn)
raise analysis_error(
"PyStackRef_FromPyObjectNew() must be assigned to an output", tkn
)

name = lhs[0].text
if not any(var.name == name for var in node.outputs):
raise analysis_error(f"PyStackRef_FromPyObjectNew() must be assigned to an output, not '{name}'", tkn)
raise analysis_error(
f"PyStackRef_FromPyObjectNew() must be assigned to an output, not '{name}'",
tkn,
)

refs[tkn] = name

return refs


def variable_used(node: parser.InstDef, name: str) -> bool:
"""Determine whether a variable with a given name is used in a node."""
return any(
token.kind == "IDENTIFIER" and token.text == name for token in node.block.tokens
)


def oparg_used(node: parser.InstDef) -> bool:
"""Determine whether `oparg` is used in a node."""
return any(
token.kind == "IDENTIFIER" and token.text == "oparg" for token in node.tokens
)


def tier_variable(node: parser.InstDef) -> int | None:
"""Determine whether a tier variable is used in a node."""
for token in node.tokens:
Expand All @@ -416,6 +438,7 @@ def tier_variable(node: parser.InstDef) -> int | None:
return int(token.text[-1])
return None


def has_error_with_pop(op: parser.InstDef) -> bool:
return (
variable_used(op, "ERROR_IF")
Expand All @@ -424,6 +447,7 @@ def has_error_with_pop(op: parser.InstDef) -> bool:
or variable_used(op, "resume_with_error")
)


def has_error_without_pop(op: parser.InstDef) -> bool:
return (
variable_used(op, "ERROR_NO_POP")
Expand Down Expand Up @@ -606,8 +630,10 @@ def stack_effect_only_peeks(instr: parser.InstDef) -> bool:
for s, other in zip(stack_inputs, instr.outputs)
)


OPARG_AND_1 = re.compile("\\(*oparg *& *1")


def effect_depends_on_oparg_1(op: parser.InstDef) -> bool:
for effect in op.inputs:
if isinstance(effect, parser.CacheEffect):
Expand All @@ -623,6 +649,7 @@ def effect_depends_on_oparg_1(op: parser.InstDef) -> bool:
return True
return False


def compute_properties(op: parser.InstDef) -> Properties:
has_free = (
variable_used(op, "PyCell_New")
Expand Down Expand Up @@ -667,7 +694,12 @@ def compute_properties(op: parser.InstDef) -> Properties:
)


def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uops: dict[str, Uop]) -> Uop:
def make_uop(
name: str,
op: parser.InstDef,
inputs: list[parser.InputEffect],
uops: dict[str, Uop],
) -> Uop:
result = Uop(
name=name,
context=op.context,
Expand All @@ -685,7 +717,9 @@ def make_uop(name: str, op: parser.InstDef, inputs: list[parser.InputEffect], uo
properties = compute_properties(op)
if properties.oparg:
# May not need oparg anymore
properties.oparg = any(token.text == "oparg" for token in op.block.tokens)
properties.oparg = any(
token.text == "oparg" for token in op.block.tokens
)
rep = Uop(
name=name_x,
context=op.context,
Expand Down Expand Up @@ -736,8 +770,10 @@ def add_op(op: parser.InstDef, uops: dict[str, Uop]) -> None:


def add_instruction(
where: lexer.Token, name: str, parts: list[Part],
instructions: dict[str, Instruction]
where: lexer.Token,
name: str,
parts: list[Part],
instructions: dict[str, Instruction],
) -> None:
instructions[name] = Instruction(where, name, parts, None)

Expand Down Expand Up @@ -781,7 +817,9 @@ def add_macro(
parts.append(Flush())
else:
if part.name not in uops:
raise analysis_error(f"No Uop named {part.name}", macro.tokens[0])
raise analysis_error(
f"No Uop named {part.name}", macro.tokens[0]
)
parts.append(uops[part.name])
case parser.CacheEffect():
parts.append(Skip(part.size))
Expand Down
7 changes: 3 additions & 4 deletions Tools/cases_generator/generators_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,13 @@ def emit_to(out: CWriter, tkn_iter: Iterator[Token], end: str) -> None:
parens -= 1
out.emit(tkn)


ReplacementFunctionType = Callable[
[Token, Iterator[Token], Uop, Stack, Instruction | None], None
]

class Emitter:

class Emitter:
out: CWriter
_replacers: dict[str, ReplacementFunctionType]

Expand Down Expand Up @@ -176,7 +177,6 @@ def decref_inputs(
else:
self.out.emit(f"PyStackRef_CLOSE({var.name});\n")


def sync_sp(
self,
tkn: Token,
Expand All @@ -190,7 +190,6 @@ def sync_sp(
next(tkn_iter)
stack.flush(self.out)


def check_eval_breaker(
self,
tkn: Token,
Expand Down Expand Up @@ -227,7 +226,6 @@ def py_stack_ref_from_py_object_new(
# unused portions of the stack to NULL.
stack.flush_single_var(self.out, target, uop.stack.outputs)


def emit_tokens(
self,
uop: Uop,
Expand All @@ -248,6 +246,7 @@ def emit_tokens(
def emit(self, txt: str | Token) -> None:
self.out.emit(txt)


def cflags(p: Properties) -> str:
flags: list[str] = []
if p.oparg:
Expand Down
1 change: 1 addition & 0 deletions Tools/cases_generator/opcode_metadata_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def emit_stack_effect_function(
def generate_stack_effect_functions(analysis: Analysis, out: CWriter) -> None:
popped_data: list[tuple[str, str]] = []
pushed_data: list[tuple[str, str]] = []

def add(inst: Instruction | PseudoInstruction) -> None:
stack = get_stack_effect(inst)
popped = (-stack.base_offset).to_c()
Expand Down
13 changes: 7 additions & 6 deletions Tools/cases_generator/optimizer_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ def emit_default(out: CWriter, uop: Uop) -> None:
else:
out.emit(f"{var.name} = sym_new_not_null(ctx);\n")

class OptimizerEmitter(Emitter):

class OptimizerEmitter(Emitter):
pass


Expand Down Expand Up @@ -139,7 +139,7 @@ def write_uop(
local = locals[var.name]
else:
local = Local.local(var)
out.emit(stack.push(local))
stack.push(local)
out.start_line()
stack.flush(out, cast_type="_Py_UopsSymbol *", extract_bits=True)
except StackError as ex:
Expand All @@ -161,8 +161,9 @@ def generate_abstract_interpreter(
out.emit("\n")
base_uop_names = set([uop.name for uop in base.uops.values()])
for abstract_uop_name in abstract.uops:
assert abstract_uop_name in base_uop_names,\
f"All abstract uops should override base uops, but {abstract_uop_name} is not."
assert (
abstract_uop_name in base_uop_names
), f"All abstract uops should override base uops, but {abstract_uop_name} is not."

for uop in base.uops.values():
override: Uop | None = None
Expand Down Expand Up @@ -192,7 +193,7 @@ def generate_abstract_interpreter(


def generate_tier2_abstract_from_files(
filenames: list[str], outfilename: str, debug: bool=False
filenames: list[str], outfilename: str, debug: bool = False
) -> None:
assert len(filenames) == 2, "Need a base file and an abstract cases file."
base = analyze_files([filenames[0]])
Expand All @@ -211,7 +212,7 @@ def generate_tier2_abstract_from_files(
)


arg_parser.add_argument("input", nargs='*', help="Abstract interpreter definition file")
arg_parser.add_argument("input", nargs="*", help="Abstract interpreter definition file")

arg_parser.add_argument(
"base", nargs="*", help="The base instruction definition file(s)"
Expand Down
Loading

0 comments on commit 357a27c

Please sign in to comment.