From 614ea0da18110b9be4192dd96f3baad695b9e888 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 28 Dec 2024 16:45:49 -0500 Subject: [PATCH] fix[venom]: fix store elimination pass (#4428) 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 --- vyper/venom/passes/store_elimination.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/vyper/venom/passes/store_elimination.py b/vyper/venom/passes/store_elimination.py index 22d4723013..a4f217505b 100644 --- a/vyper/venom/passes/store_elimination.py +++ b/vyper/venom/passes/store_elimination.py @@ -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 @@ -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)