Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Make branch extraction more intelligent #652

Merged
merged 14 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion package/version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.1.439
0.1.440
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"

[tool.poetry]
name = "pyk"
version = "0.1.439"
version = "0.1.440"
description = ""
authors = [
"Runtime Verification, Inc. <[email protected]>",
Expand Down
30 changes: 21 additions & 9 deletions src/pyk/kcfg/explore.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,14 +377,24 @@ def extend(
if self._check_abstract(node, kcfg):
return

if not kcfg.splits(target_id=node.id):
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is no longer relevant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, as I understand, it was the check preventing us from branching over and over again. But by simplifying before branching when it tries to branch a second time without having made an execution step, one of the branches will now simplify to bottom and it won't branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean by 'over and over again', when can that happen? As part of looping code?

branches = self.kcfg_semantics.extract_branches(node.cterm)
if branches:
kcfg.split_on_constraints(node.id, branches)
_LOGGER.info(
f'Found {len(branches)} branches using heuristic for node {node.id}: {shorten_hashes(node.id)}: {[self.kprint.pretty_print(bc) for bc in branches]}'
)
return
branches = self.kcfg_semantics.extract_branches(node.cterm)

branch_conditions = []
branch_cterms = []

for constraint in branches:
cterm = node.cterm.add_constraint(constraint)
nwatson22 marked this conversation as resolved.
Show resolved Hide resolved
cterm, _ = self.cterm_simplify(cterm)
Copy link
Contributor

Choose a reason for hiding this comment

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

...and here use kast_simplify?

Copy link
Contributor

Choose a reason for hiding this comment

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

and maybe put the constraint back into the cterm if that's what's easier for branch later on...

Copy link
Member Author

Choose a reason for hiding this comment

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

Using kast_simplify. I removed for now the adding of the already simplified term as the new branch nodes.

Copy link
Member

Choose a reason for hiding this comment

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

Why kast_simplify instead of cterm_simplify?

Copy link
Member

Choose a reason for hiding this comment

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

I guess the argument is performance. Have we checked on KEVM that it solves the problems we had there?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't myself know if it makes an appreciable difference to performance, but if it doesn't make any difference to have the configuration in there, it's just simpler.
I have checked and it does solve my issue with repeated branching. I'm not sure if there was a specific problem that prompted #632 or just a general observation. @PetarMax, do you have a KEVM example where this was causing problems that I could test this against?

if not cterm.is_bottom:
branch_conditions.append(constraint)
branch_cterms.append(cterm)

if len(branch_cterms) > 1 and branches:
nwatson22 marked this conversation as resolved.
Show resolved Hide resolved
kcfg.branch(node.id, zip(branch_cterms, branch_conditions, strict=True))
_LOGGER.info(
f'Found {len(branches)} branches using heuristic for node {node.id}: {shorten_hashes(node.id)}: {[self.kprint.pretty_print(bc) for bc in branches]}'
)
return

ehildenb marked this conversation as resolved.
Show resolved Hide resolved
_LOGGER.info(f'Extending KCFG from node {self.id}: {shorten_hashes(node.id)}')
_is_vacuous, depth, cterm, next_cterms, next_node_logs = self.cterm_execute(
Expand Down Expand Up @@ -437,7 +447,9 @@ def extend(

# Split on branch patterns
if any(branch_pattern.match(branch_and) for branch_pattern in branch_patterns):
kcfg.split_on_constraints(node.id, branches)
branch_cterms = [node.cterm.add_constraint(branch) for branch in branches]

kcfg.branch(node.id, zip(branch_cterms, branches, strict=True))
_LOGGER.info(
f'Found {len(branches)} branches for node {self.id}: {shorten_hashes(node.id)}: {[self.kprint.pretty_print(bc) for bc in branches]}'
)
Expand Down
9 changes: 6 additions & 3 deletions src/pyk/kcfg/kcfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -724,10 +724,13 @@ def create_ndbranch(self, source_id: NodeIdLike, ndbranches: Iterable[NodeIdLike
ndbranch = KCFG.NDBranch(self.node(source_id), tuple(self.node(nid) for nid in ndbranches))
self._ndbranches[source_id] = ndbranch

def split_on_constraints(self, source_id: NodeIdLike, constraints: Iterable[KInner]) -> list[int]:
def branch(self, source_id: NodeIdLike, splits: Iterable[tuple[CTerm, KInner]]) -> list[int]:
source = self.node(source_id)
branch_node_ids = [self.create_node(source.cterm.add_constraint(c)).id for c in constraints]
csubsts = [CSubst(constraints=flatten_label('#And', constraint)) for constraint in constraints]
branch_node_ids = []
csubsts = []
for cterm, constraint in splits:
branch_node_ids.append(self.create_node(cterm).id)
csubsts.append(CSubst(constraints=flatten_label('#And', constraint)))
self.create_split(source.id, zip(branch_node_ids, csubsts, strict=True))
return branch_node_ids

Expand Down
Loading