-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
GH-98831: Implement basic cache effects #99313
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
Changes from 1 commit
9f15c4b
6189043
f5e1aed
a8d608d
4ee85e7
873da31
e0ab5b8
882fdec
8226fb9
1aa0124
d094e42
d3d907a
0339a67
f3e7dd6
e3ff6ac
3db443a
c58a85a
756a41b
48400ac
433243a
3d51484
4d42a0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -67,7 +67,10 @@ def always_exits(block: parser.Block) -> bool: | |||||||||
return line.startswith(("goto ", "return ", "DISPATCH", "GO_TO_", "Py_UNREACHABLE()")) | ||||||||||
|
||||||||||
|
||||||||||
def write_instr(instr: InstDef, predictions: set[str], indent: str, f: TextIO, dedent: int = 0): | ||||||||||
def write_instr( | ||||||||||
instr: InstDef, predictions: set[str], indent: str, f: TextIO, dedent: int = 0 | ||||||||||
) -> int: | ||||||||||
# Returns cache offset | ||||||||||
if dedent < 0: | ||||||||||
indent += " " * -dedent | ||||||||||
# Separate stack inputs from cache inputs | ||||||||||
|
@@ -124,7 +127,7 @@ def write_instr(instr: InstDef, predictions: set[str], indent: str, f: TextIO, d | |||||||||
f.write(line) | ||||||||||
if always_exits(instr.block): | ||||||||||
# None of the rest matters | ||||||||||
return | ||||||||||
return cache_offset | ||||||||||
# Stack effect | ||||||||||
noutputs = len(outputs) | ||||||||||
diff = noutputs - ninputs | ||||||||||
|
@@ -138,9 +141,12 @@ def write_instr(instr: InstDef, predictions: set[str], indent: str, f: TextIO, d | |||||||||
# Cache effect | ||||||||||
if cache_offset: | ||||||||||
f.write(f"{indent} next_instr += {cache_offset};\n") | ||||||||||
return cache_offset | ||||||||||
|
||||||||||
|
||||||||||
def write_cases(f: TextIO, instrs: list[InstDef], supers: list[parser.Super]): | ||||||||||
def write_cases( | ||||||||||
f: TextIO, instrs: list[InstDef], supers: list[parser.Super] | ||||||||||
) -> dict[str, tuple[int, int, int]]: | ||||||||||
predictions: set[str] = set() | ||||||||||
for instr in instrs: | ||||||||||
for target in re.findall(RE_PREDICTED, instr.block.text): | ||||||||||
|
@@ -149,12 +155,14 @@ def write_cases(f: TextIO, instrs: list[InstDef], supers: list[parser.Super]): | |||||||||
f.write(f"// This file is generated by {os.path.relpath(__file__)}\n") | ||||||||||
f.write(f"// Do not edit!\n") | ||||||||||
instr_index: dict[str, InstDef] = {} | ||||||||||
effects_table: dict[str, tuple[int, int, int]] = {} # name -> (ninputs, noutputs, cache_offset) | ||||||||||
for instr in instrs: | ||||||||||
instr_index[instr.name] = instr | ||||||||||
f.write(f"\n{indent}TARGET({instr.name}) {{\n") | ||||||||||
if instr.name in predictions: | ||||||||||
f.write(f"{indent} PREDICTED({instr.name});\n") | ||||||||||
write_instr(instr, predictions, indent, f) | ||||||||||
cache_offset = write_instr(instr, predictions, indent, f) | ||||||||||
effects_table[instr.name] = len(instr.inputs), len(instr.outputs), cache_offset | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm. This is a bit weird because it treats caches and stack items the same, which doesn't make much sense. It seems to me we'd want something that captures just stack effect and cache size:
Suggested change
(This will probably get more complicated as stack effects start getting more complicated...) |
||||||||||
if not always_exits(instr.block): | ||||||||||
f.write(f"{indent} DISPATCH();\n") | ||||||||||
# Write trailing '}' | ||||||||||
|
@@ -173,6 +181,8 @@ def write_cases(f: TextIO, instrs: list[InstDef], supers: list[parser.Super]): | |||||||||
f.write(f"{indent} DISPATCH();\n") | ||||||||||
f.write(f"{indent}}}\n") | ||||||||||
|
||||||||||
return effects_table | ||||||||||
|
||||||||||
|
||||||||||
def main(): | ||||||||||
args = arg_parser.parse_args() | ||||||||||
|
@@ -193,12 +203,28 @@ def main(): | |||||||||
file=sys.stderr, | ||||||||||
) | ||||||||||
with eopen(args.output, "w") as f: | ||||||||||
write_cases(f, instrs, supers) | ||||||||||
effects_table = write_cases(f, instrs, supers) | ||||||||||
if not args.quiet: | ||||||||||
print( | ||||||||||
f"Wrote {ninstrs + nsupers} instructions to {args.output}", | ||||||||||
file=sys.stderr, | ||||||||||
) | ||||||||||
# Check that families have consistent effects | ||||||||||
gvanrossum marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
errors = 0 | ||||||||||
for family in families: | ||||||||||
head = effects_table[family.members[0]] | ||||||||||
for member in family.members: | ||||||||||
if effects_table[member] != head: | ||||||||||
errors += 1 | ||||||||||
print( | ||||||||||
f"Family {family.name!r} has inconsistent effects (inputs, outputs, cache units):", | ||||||||||
file=sys.stderr, | ||||||||||
) | ||||||||||
print( | ||||||||||
f" {family.members[0]} = {head}; {member} = {effects_table[member]}", | ||||||||||
) | ||||||||||
if errors: | ||||||||||
sys.exit(1) | ||||||||||
|
||||||||||
|
||||||||||
if __name__ == "__main__": | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)