From 9ed6b54249180bda8ea2ea09edca63dd22afe06e Mon Sep 17 00:00:00 2001
From: whitequark <whitequark@whitequark.org>
Date: Thu, 31 Dec 2015 17:07:31 +0800
Subject: [PATCH] transforms.cfg_simplifier: remove.

Specifically, the following idiom broke our CFGSimplifier:

b1:
  branch_if %cond, %b2, %b3
b2:
  branch %b3
b3:
  %v = phi [ %b2 => 0, %b3 => 1 ]

This is very annoying to properly handle in general case, and also
this (specifically %b2) is exactly what broke LocalAccessValidator
in the first place. So, remove CFGSimplifier altogether.

Instead, teach LocalAccessValidator how to look through predecessors
in order to get a location.
---
 artiq/compiler/module.py                    |  2 --
 artiq/compiler/transforms/__init__.py       |  1 -
 artiq/compiler/transforms/cfg_simplifier.py | 35 ---------------------
 artiq/compiler/validators/local_access.py   | 20 +++++++++---
 4 files changed, 16 insertions(+), 42 deletions(-)
 delete mode 100644 artiq/compiler/transforms/cfg_simplifier.py

diff --git a/artiq/compiler/module.py b/artiq/compiler/module.py
index afeb21a69..59d1a289f 100644
--- a/artiq/compiler/module.py
+++ b/artiq/compiler/module.py
@@ -55,7 +55,6 @@ class Module:
         artiq_ir_generator = transforms.ARTIQIRGenerator(engine=self.engine,
                                                          module_name=src.name,
                                                          ref_period=ref_period)
-        cfg_simplifier = transforms.CFGSimplifier(engine=self.engine)
         dead_code_eliminator = transforms.DeadCodeEliminator(engine=self.engine)
         local_access_validator = validators.LocalAccessValidator(engine=self.engine)
         devirtualization = analyses.Devirtualization()
@@ -71,7 +70,6 @@ class Module:
         devirtualization.visit(src.typedtree)
         self.artiq_ir = artiq_ir_generator.visit(src.typedtree)
         artiq_ir_generator.annotate_calls(devirtualization)
-        cfg_simplifier.process(self.artiq_ir)
         dead_code_eliminator.process(self.artiq_ir)
         interleaver.process(self.artiq_ir)
         local_access_validator.process(self.artiq_ir)
diff --git a/artiq/compiler/transforms/__init__.py b/artiq/compiler/transforms/__init__.py
index d672d3604..665fd3ea1 100644
--- a/artiq/compiler/transforms/__init__.py
+++ b/artiq/compiler/transforms/__init__.py
@@ -4,6 +4,5 @@ from .int_monomorphizer import IntMonomorphizer
 from .iodelay_estimator import IODelayEstimator
 from .artiq_ir_generator import ARTIQIRGenerator
 from .dead_code_eliminator import DeadCodeEliminator
-from .cfg_simplifier import CFGSimplifier
 from .llvm_ir_generator import LLVMIRGenerator
 from .interleaver import Interleaver
diff --git a/artiq/compiler/transforms/cfg_simplifier.py b/artiq/compiler/transforms/cfg_simplifier.py
deleted file mode 100644
index 22b915884..000000000
--- a/artiq/compiler/transforms/cfg_simplifier.py
+++ /dev/null
@@ -1,35 +0,0 @@
-"""
-:class:`CFGSimplifier` is a simple control flow graph
-simplification transform: it removes empty basic blocks.
-"""
-
-from .. import ir
-
-class CFGSimplifier:
-    def __init__(self, engine):
-        self.engine = engine
-
-    def process(self, functions):
-        for func in functions:
-            self.process_function(func)
-
-    def process_function(self, func):
-        for block in list(func.basic_blocks):
-            if len(block.instructions) == 1 and \
-                    isinstance(block.terminator(), ir.Branch):
-                successor, = block.successors()
-
-                for insn in set(block.uses):
-                    if isinstance(insn, ir.BranchIf) and \
-                            ((insn.if_true() == block and insn.if_false() == successor) or
-                             (insn.if_true() == successor and insn.if_false() == block)):
-                        # Our IR doesn't tolerate branch_if %c, %b, %b
-                        insn.replace_with(ir.Branch(successor))
-                    elif isinstance(insn, ir.Phi):
-                        incoming_value = insn.incoming_value_for_block(block)
-                        insn.remove_incoming_block(block)
-                        for pred in block.predecessors():
-                            insn.add_incoming(incoming_value, pred)
-
-                block.replace_all_uses_with(successor)
-                block.erase()
diff --git a/artiq/compiler/validators/local_access.py b/artiq/compiler/validators/local_access.py
index fed87092b..55ed7bd0d 100644
--- a/artiq/compiler/validators/local_access.py
+++ b/artiq/compiler/validators/local_access.py
@@ -143,11 +143,23 @@ class LocalAccessValidator:
 
     def _uninitialized_access(self, insn, var_name, pred_at_fault):
         if pred_at_fault is not None:
+            visited = set()
+            possible_preds = [pred_at_fault]
+
             uninitialized_loc = None
-            for pred_insn in reversed(pred_at_fault.instructions):
-                if pred_insn.loc is not None:
-                    uninitialized_loc = pred_insn.loc.begin()
-                    break
+            while uninitialized_loc is None:
+                possible_pred = possible_preds.pop(0)
+                visited.add(possible_pred)
+
+                for pred_insn in reversed(possible_pred.instructions):
+                    if pred_insn.loc is not None:
+                        uninitialized_loc = pred_insn.loc.begin()
+                        break
+
+                for block in possible_pred.predecessors():
+                    if block not in visited:
+                        possible_preds.append(block)
+
             assert uninitialized_loc is not None
 
             note = diagnostic.Diagnostic("note",