Skip to content

Commit

Permalink
fix[venom]: fix store elimination pass (#4428)
Browse files Browse the repository at this point in the history
this commit fixes the store elimination pass by updating the dfg
in-place instead of relying on a stale dfg.

this currently results in no bytecode changes. previously this
was undetected because the order of items in the dfg happens to be
"well-behaved", but if the dfg is built using a traversal of basic
blocks in a different order (as may happen in upcoming passes), it can
result in store instructions failing to be eliminated.

note that we haven't rebuilt the dfg properly because `dfg.outputs`
is invalid after this pass. we could modify `dfg.outputs` in place, but
that results in a bytecode regression.

this commit also removes the dependency on CFGAnalysis as it is not
actually needed by the pass.

---------

Co-authored-by: Harry Kalogirou <[email protected]>
  • Loading branch information
charles-cooper and harkal authored Dec 28, 2024
1 parent e9ac7cd commit 614ea0d
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions vyper/venom/passes/store_elimination.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, LivenessAnalysis
from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis
from vyper.venom.basicblock import IRVariable
from vyper.venom.passes.base_pass import IRPass

Expand All @@ -13,31 +13,33 @@ class StoreElimination(IRPass):
# with LoadElimination

def run_pass(self):
self.analyses_cache.request_analysis(CFGAnalysis)
dfg = self.analyses_cache.request_analysis(DFGAnalysis)
self.dfg = self.analyses_cache.request_analysis(DFGAnalysis)

for var, inst in dfg.outputs.items():
for var, inst in self.dfg.outputs.items():
if inst.opcode != "store":
continue
self._process_store(dfg, inst, var, inst.operands[0])
self._process_store(inst, var, inst.operands[0])

self.analyses_cache.invalidate_analysis(LivenessAnalysis)
self.analyses_cache.invalidate_analysis(DFGAnalysis)

def _process_store(self, dfg, inst, var: IRVariable, new_var: IRVariable):
def _process_store(self, inst, var: IRVariable, new_var: IRVariable):
"""
Process store instruction. If the variable is only used by a load instruction,
forward the variable to the load instruction.
"""
if any([inst.opcode == "phi" for inst in dfg.get_uses(new_var)]):
if any([inst.opcode == "phi" for inst in self.dfg.get_uses(new_var)]):
return

uses = dfg.get_uses(var)
uses = self.dfg.get_uses(var)
if any([inst.opcode == "phi" for inst in uses]):
return
for use_inst in uses:
for use_inst in uses.copy():
for i, operand in enumerate(use_inst.operands):
if operand == var:
use_inst.operands[i] = new_var

self.dfg.add_use(new_var, use_inst)
self.dfg.remove_use(var, use_inst)

inst.parent.remove_instruction(inst)

0 comments on commit 614ea0d

Please sign in to comment.