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

fix[venom]: fix invalid phis after SCCP #4181

Merged
Merged
Show file tree
Hide file tree
Changes from 13 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
42 changes: 40 additions & 2 deletions vyper/venom/passes/sccp/sccp.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,18 @@ class SCCP(IRPass):
uses: dict[IRVariable, OrderedSet[IRInstruction]]
lattice: Lattice
work_list: list[WorkListItem]
cfg_dirty: bool
cfg_in_exec: dict[IRBasicBlock, OrderedSet[IRBasicBlock]]

cfg_dirty: bool
# list of basic blocks whose cfg_in was modified
cfg_in_modified: OrderedSet[IRBasicBlock]

def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction):
super().__init__(analyses_cache, function)
self.lattice = {}
self.work_list: list[WorkListItem] = []
self.cfg_dirty = False
self.cfg_in_modified = OrderedSet()

def run_pass(self):
self.fn = self.function
Expand All @@ -74,7 +78,9 @@ def run_pass(self):

# self._propagate_variables()

self.analyses_cache.invalidate_analysis(CFGAnalysis)
if self.cfg_dirty:
self.analyses_cache.force_analysis(CFGAnalysis)
self._fix_phi_nodes()

def _calculate_sccp(self, entry: IRBasicBlock):
"""
Expand Down Expand Up @@ -304,7 +310,11 @@ def _replace_constants(self, inst: IRInstruction):
target = inst.operands[1]
inst.opcode = "jmp"
inst.operands = [target]

self.cfg_dirty = True
for bb in inst.parent.cfg_out:
if bb.label == target:
self.cfg_in_modified.add(bb)

elif inst.opcode in ("assert", "assert_unreachable"):
lat = self._eval_from_lattice(inst.operands[0])
Expand All @@ -329,6 +339,34 @@ def _replace_constants(self, inst: IRInstruction):
if isinstance(lat, IRLiteral):
inst.operands[i] = lat

def _fix_phi_nodes(self):
charles-cooper marked this conversation as resolved.
Show resolved Hide resolved
# fix basic blocks whose cfg in was changed
# maybe this should really be done in _visit_phi
needs_sort = False

for bb in self.fn.get_basic_blocks():
cfg_in_labels = OrderedSet(in_bb.label for in_bb in bb.cfg_in)

for inst in bb.instructions:
if inst.opcode != "phi":
break
needs_sort |= self._fix_phi_inst(inst, cfg_in_labels)

# move phi instructions to the top of the block
if needs_sort:
bb.instructions.sort(key=lambda inst: inst.opcode != "phi")

def _fix_phi_inst(self, inst: IRInstruction, cfg_in_labels: OrderedSet):
operands = [op for label, op in inst.phi_operands if label in cfg_in_labels]

if len(operands) != 1:
return False

assert inst.output is not None
inst.opcode = "store"
inst.operands = operands
return True


def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem:
if x == LatticeEnum.TOP:
Expand Down
16 changes: 7 additions & 9 deletions vyper/venom/passes/simplify_cfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,21 @@ class SimplifyCFGPass(IRPass):
visited: OrderedSet

def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock):
a.instructions.pop()
a.instructions.pop() # pop terminating instructoin
for inst in b.instructions:
assert inst.opcode != "phi", "Not implemented yet"
if inst.opcode == "phi":
a.instructions.insert(0, inst)
else:
inst.parent = a
a.instructions.append(inst)
assert inst.opcode != "phi", f"Instruction should never be phi {b}"
inst.parent = a
a.instructions.append(inst)

# Update CFG
a.cfg_out = b.cfg_out
if len(b.cfg_out) > 0:
next_bb = b.cfg_out.first()

for next_bb in a.cfg_out:
next_bb.remove_cfg_in(b)
next_bb.add_cfg_in(a)

for inst in next_bb.instructions:
# assume phi instructions are at beginning of bb
if inst.opcode != "phi":
break
inst.operands[inst.operands.index(b.label)] = a.label
Expand Down
Loading