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

GH-120024: Tidy up case generator code a bit. #122780

Merged
merged 6 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading