Implement better magic to handle changing control dependencies when welding blocks
[libfirm] / ir / opt / ldstopt.c
index e1ec07a..2e9aeea 100644 (file)
@@ -36,6 +36,7 @@
 # include "irflag_t.h"
 # include "array.h"
 # include "irhooks.h"
+# include "irtools.h"
 # include "opt_polymorphy.h"
 
 #ifdef DO_CACHEOPT
 
 #define MAX_PROJ       IMAX(pn_Load_max, pn_Store_max)
 
+enum changes_t {
+  DF_CHANGED = 1,       /**< data flow changed */
+  CF_CHANGED = 2,       /**< control flow changed */
+};
+
 /**
  * walker environment
  */
 typedef struct _walk_env_t {
   struct obstack obst;         /**< list of all stores */
-  int changes;
+  unsigned changes;             /**< a bitmask of graph changes */
 } walk_env_t;
 
 /**
@@ -70,6 +76,7 @@ typedef struct _ldst_info_t {
   ir_node  *exc_block;          /**< the exception block if available */
   int      exc_idx;             /**< predecessor index in the exception block */
   unsigned flags;               /**< flags */
+  unsigned visited;             /**< visited counter for breaking loops */
 } ldst_info_t;
 
 /**
@@ -77,7 +84,7 @@ typedef struct _ldst_info_t {
  */
 enum block_flags_t {
   BLOCK_HAS_COND = 1,      /**< Block has conditional control flow */
-  BLOCK_HAS_EXC  = 2       /**< Block has exceptionl control flow */
+  BLOCK_HAS_EXC  = 2       /**< Block has exceptional control flow */
 };
 
 /**
@@ -87,13 +94,12 @@ typedef struct _block_info_t {
   unsigned flags;               /**< flags for the block */
 } block_info_t;
 
-/**
- * walker, clears all links first
- */
-static void init_links(ir_node *n, void *env)
-{
-  set_irn_link(n, NULL);
-}
+/** the master visited flag for loop detection. */
+static unsigned master_visited = 0;
+
+#define INC_MASTER()       ++master_visited
+#define MARK_NODE(info)    (info)->visited = master_visited
+#define NODE_VISITED(info) (info)->visited >= master_visited
 
 /**
  * get the Load/Store info of a node
@@ -132,7 +138,7 @@ static block_info_t *get_block_info(ir_node *node, walk_env_t *env)
 /**
  * update the projection info for a Load/Store
  */
-static int update_projs(ldst_info_t *info, ir_node *proj)
+static unsigned update_projs(ldst_info_t *info, ir_node *proj)
 {
   long nr = get_Proj_proj(proj);
 
@@ -141,7 +147,7 @@ static int update_projs(ldst_info_t *info, ir_node *proj)
   if (info->projs[nr]) {
     /* there is already one, do CSE */
     exchange(proj, info->projs[nr]);
-    return 1;
+    return DF_CHANGED;
   }
   else {
     info->projs[nr] = proj;
@@ -150,9 +156,13 @@ static int update_projs(ldst_info_t *info, ir_node *proj)
 }
 
 /**
- * update the exception block info for a Load/Store
+ * update the exception block info for a Load/Store node.
+ *
+ * @param info   the load/store info struct
+ * @param block  the exception handler block for this load/store
+ * @param pos    the control flow input of the block
  */
-static int update_exc(ldst_info_t *info, ir_node *block, int pos)
+static unsigned update_exc(ldst_info_t *info, ir_node *block, int pos)
 {
   assert(info->exc_block == NULL && "more than one exception block found");
 
@@ -161,18 +171,18 @@ static int update_exc(ldst_info_t *info, ir_node *block, int pos)
   return 0;
 }
 
-#define get_irn_out_n(node)     (unsigned)get_irn_link(node)
-#define set_irn_out_n(node, n)  set_irn_link(adr, (void *)(n))
+#define get_irn_out_n(node)     (unsigned)PTR_TO_INT(get_irn_link(node))
+#define set_irn_out_n(node, n)  set_irn_link(adr, INT_TO_PTR(n))
 
 /**
  * walker, collects all Load/Store/Proj nodes
  *
- * walks form Start -> End
+ * walks from Start -> End
  */
 static void collect_nodes(ir_node *node, void *env)
 {
   ir_op       *op = get_irn_op(node);
-  ir_node     *pred;
+  ir_node     *pred, *blk, *pred_blk;
   ldst_info_t *ldst_info;
   walk_env_t  *wenv = env;
 
@@ -194,6 +204,19 @@ static void collect_nodes(ir_node *node, void *env)
 
         ldst_info->flags |= LDST_VISITED;
       }
+
+      /*
+       * Place the Proj's to the same block as the
+       * predecessor Load. This is always ok and prevents
+       * "non-SSA" form after optimizations if the Proj
+       * is in a wrong block.
+       */
+      blk      = get_nodes_block(node);
+      pred_blk = get_nodes_block(pred);
+      if (blk != pred_blk) {
+        wenv->changes |= DF_CHANGED;
+        set_nodes_block(node, pred_blk);
+      }
     }
     else if (op == op_Store) {
       ldst_info = get_ldst_info(pred, wenv);
@@ -206,12 +229,25 @@ static void collect_nodes(ir_node *node, void *env)
 
         ldst_info->flags |= LDST_VISITED;
       }
+
+      /*
+       * Place the Proj's to the same block as the
+       * predecessor Store. This is always ok and prevents
+       * "non-SSA" form after optimizations if the Proj
+       * is in a wrong block.
+       */
+      blk      = get_nodes_block(node);
+      pred_blk = get_nodes_block(pred);
+      if (blk != pred_blk) {
+        wenv->changes |= DF_CHANGED;
+        set_nodes_block(node, pred_blk);
+      }
     }
   }
   else if (op == op_Block) { /* check, if it's an exception block */
-    int i, n;
+    int i;
 
-    for (i = 0, n = get_Block_n_cfgpreds(node); i < n; ++i) {
+    for (i = get_Block_n_cfgpreds(node) - 1; i >= 0; --i) {
       ir_node      *pred_block;
       block_info_t *bl_info;
 
@@ -219,15 +255,15 @@ static void collect_nodes(ir_node *node, void *env)
 
       /* ignore Bad predecessors, they will be removed later */
       if (is_Bad(pred))
-       continue;
+        continue;
 
       pred_block = get_nodes_block(pred);
       bl_info    = get_block_info(pred_block, wenv);
 
       if (is_fragile_op(pred))
-       bl_info->flags |= BLOCK_HAS_EXC;
-      else if (is_forking_op(pred))
-       bl_info->flags |= BLOCK_HAS_COND;
+             bl_info->flags |= BLOCK_HAS_EXC;
+      else if (is_irn_forking(pred))
+        bl_info->flags |= BLOCK_HAS_COND;
 
       if (get_irn_op(pred) == op_Load || get_irn_op(pred) == op_Store) {
         ldst_info = get_ldst_info(pred, wenv);
@@ -250,17 +286,17 @@ static entity *find_constant_entity(ir_node *ptr)
       return get_SymConst_entity(ptr);
     }
     else if (op == op_Sel) {
-      entity *ent = get_Sel_entity(ptr);
-      type *tp    = get_entity_owner(ent);
+      entity  *ent = get_Sel_entity(ptr);
+      ir_type *tp  = get_entity_owner(ent);
 
-      /* Do not fiddle about polymorphy. */
+      /* Do not fiddle with polymorphism. */
       if (is_Class_type(get_entity_owner(ent)) &&
-         ((get_entity_n_overwrites(ent)    != 0) ||
-          (get_entity_n_overwrittenby(ent) != 0)   ) )
-       return NULL;
+          ((get_entity_n_overwrites(ent)    != 0) ||
+           (get_entity_n_overwrittenby(ent) != 0)   ) )
+        return NULL;
 
       if (variability_constant == get_entity_variability(ent))
-       return ent;
+        return ent;
 
       if (is_Array_type(tp)) {
         /* check bounds */
@@ -301,29 +337,48 @@ static entity *find_constant_entity(ir_node *ptr)
   }
 }
 
+/**
+ * Return the Selection index of a Sel node from dimension n
+ */
 static long get_Sel_array_index_long(ir_node *n, int dim) {
   ir_node *index = get_Sel_index(n, dim);
   assert(get_irn_op(index) == op_Const);
   return get_tarval_long(get_Const_tarval(index));
 }
 
+/**
+ * Returns the accessed component graph path for an
+ * node computing an address.
+ *
+ * @param ptr    the node computing the address
+ * @param depth  current depth in steps upward from the root
+ *               of the address
+ */
 static compound_graph_path *rec_get_accessed_path(ir_node *ptr, int depth) {
   compound_graph_path *res = NULL;
   entity              *root, *field;
   int                 path_len, pos;
 
   if (get_irn_op(ptr) == op_SymConst) {
+    /* a SymConst. If the depth is 0, this is an access to a global
+     * entity and we don't need a component path, else we know
+     * at least it's length.
+     */
     assert(get_SymConst_kind(ptr) == symconst_addr_ent);
     root = get_SymConst_entity(ptr);
-    res = depth == 0 ? NULL : new_compound_graph_path(get_entity_type(root), depth);
+    res = (depth == 0) ? NULL : new_compound_graph_path(get_entity_type(root), depth);
   }
   else {
     assert(get_irn_op(ptr) == op_Sel);
+    /* it's a Sel, go up until we find the root */
     res = rec_get_accessed_path(get_Sel_ptr(ptr), depth+1);
-    field = get_Sel_entity(ptr);
+
+    /* fill up the step in the path at the current position */
+    field    = get_Sel_entity(ptr);
     path_len = get_compound_graph_path_length(res);
-    pos = path_len - depth - 1;
+    pos      = path_len - depth - 1;
     set_compound_graph_path_node(res, pos, field);
+
     if (is_Array_type(get_entity_owner(field))) {
       assert(get_Sel_n_indexs(ptr) == 1 && "multi dim arrays not implemented");
       set_compound_graph_path_array_index(res, pos, get_Sel_array_index_long(ptr, 0));
@@ -332,56 +387,61 @@ static compound_graph_path *rec_get_accessed_path(ir_node *ptr, int depth) {
   return res;
 }
 
-
 /** Returns an access path or NULL.  The access path is only
- *  valid, if the graph is in phase_high and _no_ address computation is used. */
+ *  valid, if the graph is in phase_high and _no_ address computation is used.
+ */
 static compound_graph_path *get_accessed_path(ir_node *ptr) {
   return rec_get_accessed_path(ptr, 0);
 }
 
-
 /**
  * optimize a Load
  */
-static int optimize_load(ir_node *load)
+static unsigned optimize_load(ir_node *load)
 {
   ldst_info_t *info = get_irn_link(load);
   ir_mode *load_mode = get_Load_mode(load);
   ir_node *pred, *mem, *ptr, *new_node;
   entity *ent;
-  int res = 0;
+  unsigned res = 0;
+
+  /* do NOT touch volatile loads for now */
+  if (get_Load_volatility(load) == volatility_is_volatile)
+    return 0;
 
   /* the address of the load to be optimized */
   ptr = get_Load_ptr(load);
 
   /*
-   * Check if we can remove the exception form a Load:
-   * this can be done, if the address is from an Sel(Alloc) and
-   * the Sel type is a subtype of the alloc type.
+   * Check if we can remove the exception from a Load:
+   * This can be done, if the address is from an Sel(Alloc) and
+   * the Sel type is a subtype of the allocated type.
    *
    * This optimizes some often used OO constructs,
    * like x = new O; x->t;
    */
   if (info->projs[pn_Load_X_except]) {
-    if (get_irn_op(ptr) == op_Sel) {
+    if (is_Sel(ptr)) {
       ir_node *mem = get_Sel_mem(ptr);
 
-      if (get_irn_op(mem) == op_Alloc) {
+      if (get_irn_op(skip_Proj(mem)) == op_Alloc) {
         /* ok, check the types */
-        entity *ent  = get_Sel_entity(ptr);
-        type *s_type = get_entity_type(ent);
-        type *a_type = get_Alloc_type(mem);
+        entity  *ent    = get_Sel_entity(ptr);
+        ir_type *s_type = get_entity_type(ent);
+        ir_type *a_type = get_Alloc_type(mem);
 
-        if (is_subclass_of(s_type, a_type)) {
+        if (is_SubClass_of(s_type, a_type)) {
           /* ok, condition met: there can't be an exception because
-           * alloc guarantees that enough memory was allocated */
+           * Alloc guarantees that enough memory was allocated */
 
           exchange(info->projs[pn_Load_X_except], new_Bad());
           info->projs[pn_Load_X_except] = NULL;
+          res |= CF_CHANGED;
         }
       }
     }
-    else if (get_irn_op(ptr) == op_Alloc) {
+    else if ((get_irn_op(skip_Proj(ptr)) == op_Alloc) ||
+            ((get_irn_op(ptr) == op_Cast) && (get_irn_op(skip_Proj(get_Cast_op(ptr))) == op_Alloc))) {
       /* simple case: a direct load after an Alloc. Firm Alloc throw
        * an exception in case of out-of-memory. So, there is no way for an
        * exception in this load.
@@ -389,26 +449,22 @@ static int optimize_load(ir_node *load)
        */
        exchange(info->projs[pn_Load_X_except], new_Bad());
        info->projs[pn_Load_X_except] = NULL;
+       res |= CF_CHANGED;
     }
   }
 
-  /* do NOT touch volatile loads for now */
-  if (get_Load_volatility(load) == volatility_is_volatile)
-    return 0;
+  /* the mem of the Load. Must still be returned after optimization */
+  mem  = get_Load_mem(load);
 
   if (! info->projs[pn_Load_res] && ! info->projs[pn_Load_X_except]) {
     /* a Load which value is neither used nor exception checked, remove it */
-    mem  = get_Load_mem(load);
     exchange(info->projs[pn_Load_M], mem);
 
-    return 1;
+    return res | DF_CHANGED;
   }
 
-  /* the mem of the Load. Must still be returned after optimization */
-  mem  = get_Load_mem(load);
-
   /* Load from a constant polymorphic field, where we can resolve
-     polymorphy. */
+     polymorphism. */
   new_node = transform_node_Load(load);
   if (new_node != load) {
     if (info->projs[pn_Load_M]) {
@@ -421,7 +477,7 @@ static int optimize_load(ir_node *load)
     }
     if (info->projs[pn_Load_res])
       exchange(info->projs[pn_Load_res], new_node);
-    return 1;
+    return res | DF_CHANGED;
   }
 
   /* check if we can determine the entity that will be loaded */
@@ -436,6 +492,7 @@ static int optimize_load(ir_node *load)
       if (info->projs[pn_Load_X_except]) {
         exchange(info->projs[pn_Load_X_except], new_Bad());
         info->projs[pn_Load_X_except] = NULL;
+        res |= CF_CHANGED;
       }
 
       if (variability_constant == get_entity_variability(ent)
@@ -449,43 +506,30 @@ static int optimize_load(ir_node *load)
          */
 
         /* no memory */
-        if (info->projs[pn_Load_M])
+        if (info->projs[pn_Load_M]) {
           exchange(info->projs[pn_Load_M], mem);
+          res |= DF_CHANGED;
+        }
 
         /* no result :-) */
         if (info->projs[pn_Load_res]) {
           if (is_atomic_entity(ent)) {
-            ir_node *c = copy_const_value(get_atomic_ent_value(ent));
+            ir_node *c = copy_const_value(get_irn_dbg_info(load), get_atomic_ent_value(ent));
 
             DBG_OPT_RC(load, c);
             exchange(info->projs[pn_Load_res], c);
-
-            return 1;
+            return DF_CHANGED | res;
           }
         }
       }
       else if (variability_constant == get_entity_variability(ent)) {
-        compound_graph_path *path;
+        compound_graph_path *path = get_accessed_path(ptr);
 
-        printf(">>>>>>>>>>>>> Found access to constant entity %s in function %s\n", get_entity_name(ent),
-        get_entity_name(get_irg_entity(current_ir_graph)));
-        printf("  load: "); DDMN(load);
-        printf("  ptr:  "); DDMN(ptr);
-
-        path = get_accessed_path(ptr);
         if (path) {
           ir_node *c;
 
           assert(is_proper_compound_graph_path(path, get_compound_graph_path_length(path)-1));
-          c = get_compound_ent_value_by_path(ent, path);
-
-          printf("  cons: "); DDMN(c);
-
-          if (info->projs[pn_Load_M])
-            exchange(info->projs[pn_Load_M], mem);
-          if (info->projs[pn_Load_res])
-            exchange(info->projs[pn_Load_res], copy_const_value(c));
-
+          /*
           {
             int j;
             for (j = 0; j < get_compound_graph_path_length(path); ++j) {
@@ -496,57 +540,91 @@ static int optimize_load(ir_node *load)
             }
             printf("\n");
           }
-        } else
-          printf("cannot optimize.\n");
-      }
+          */
+
+          c = get_compound_ent_value_by_path(ent, path);
+          free_compound_graph_path(path);
 
-      /* we changed the irg, but try further */
-      res = 1;
+          /* printf("  cons: "); DDMN(c); */
+
+          if (info->projs[pn_Load_M]) {
+            exchange(info->projs[pn_Load_M], mem);
+            res |= DF_CHANGED;
+          }
+          if (info->projs[pn_Load_res]) {
+            exchange(info->projs[pn_Load_res], copy_const_value(get_irn_dbg_info(load), c));
+            return res | DF_CHANGED;
+          }
+        }
+        else {
+          /*  We can not determine a correct access path.  E.g., in jack, we load
+                   a byte from an object to generate an exception.   Happens in test program
+                   Reflectiontest.
+          printf(">>>>>>>>>>>>> Found access to constant entity %s in function %s\n", get_entity_name(ent),
+           get_entity_name(get_irg_entity(current_ir_graph)));
+          printf("  load: "); DDMN(load);
+          printf("  ptr:  "); DDMN(ptr);
+          */
+        }
+      }
     }
   }
 
   /* Check, if the address of this load is used more than once.
    * If not, this load cannot be removed in any case. */
   if (get_irn_out_n(ptr) <= 1)
-    return 0;
+    return res;
 
-  /* follow the memory chain as long as there are only Loads */
-  for (pred = skip_Proj(mem); ; pred = skip_Proj(get_Load_mem(pred))) {
+  /*
+   * follow the memory chain as long as there are only Loads
+   * and try to replace current Load or Store by a previous one.
+   * Note that in unreachable loops it might happen that we reach
+   * load again, as well as we can fall into a cycle.
+   * We break such cycles using a special visited flag.
+   */
+  INC_MASTER();
+  for (pred = skip_Proj(mem); load != pred; pred = skip_Proj(get_Load_mem(pred))) {
+    ldst_info_t *pred_info = get_irn_link(pred);
 
     /*
      * BEWARE: one might think that checking the modes is useless, because
      * if the pointers are identical, they refer to the same object.
-     * This is only true in strong typed languages, not is C were the following
-     * is possible a = *(type1 *)p; b = *(type2 *)p ...
+     * This is only true in strong typed languages, not in C were the following
+     * is possible a = *(ir_type1 *)p; b = *(ir_type2 *)p ...
      */
 
     if (get_irn_op(pred) == op_Store && get_Store_ptr(pred) == ptr &&
         get_irn_mode(get_Store_value(pred)) == load_mode) {
-      ldst_info_t *pred_info = get_irn_link(pred);
-
       /*
        * a Load immediately after a Store -- a read after write.
        * We may remove the Load, if both Load & Store does not have an exception handler
        * OR they are in the same block. In the latter case the Load cannot
        * throw an exception when the previous Store was quiet.
        *
-       * Why we need to check for Store Exc? If the Store cannot be executed (ROM)
-       * the exception handler might simply jump into the load block :-(
+       * Why we need to check for Store Exception? If the Store cannot
+       * be executed (ROM) the exception handler might simply jump into
+       * the load block :-(
        * We could make it a little bit better if we would know that the exception
        * handler of the Store jumps directly to the end...
        */
       if ((!pred_info->projs[pn_Store_X_except] && !info->projs[pn_Load_X_except]) ||
           get_nodes_block(load) == get_nodes_block(pred)) {
-        DBG_OPT_RAW(load, pred);
-        exchange( info->projs[pn_Load_res], get_Store_value(pred) );
+        ir_node *value = get_Store_value(pred);
 
+        DBG_OPT_RAW(load, value);
         if (info->projs[pn_Load_M])
           exchange(info->projs[pn_Load_M], mem);
 
         /* no exception */
-        if (info->projs[pn_Load_X_except])
+        if (info->projs[pn_Load_X_except]) {
           exchange( info->projs[pn_Load_X_except], new_Bad());
-        return 1;
+          res |= CF_CHANGED;
+        }
+
+        if (info->projs[pn_Load_res])
+          exchange(info->projs[pn_Load_res], value);
+
+        return res | DF_CHANGED;
       }
     }
     else if (get_irn_op(pred) == op_Load && get_Load_ptr(pred) == ptr &&
@@ -557,17 +635,17 @@ static int optimize_load(ir_node *load)
        * OR they are in the same block. In the later case the Load cannot
        * throw an exception when the previous Load was quiet.
        *
-       * Here, there is no need to check if the previos Load has an exception hander because
-       * they would have exact the same exception...
+       * Here, there is no need to check if the previous Load has an exception
+       * hander because they would have exact the same exception...
        */
       if (! info->projs[pn_Load_X_except] || get_nodes_block(load) == get_nodes_block(pred)) {
-        ldst_info_t *pred_info = get_irn_link(pred);
-
         DBG_OPT_RAR(load, pred);
 
         if (pred_info->projs[pn_Load_res]) {
           /* we need a data proj from the previous load for this optimization */
-          exchange( info->projs[pn_Load_res], pred_info->projs[pn_Load_res] );
+          if (info->projs[pn_Load_res])
+            exchange(info->projs[pn_Load_res], pred_info->projs[pn_Load_res]);
+
           if (info->projs[pn_Load_M])
             exchange(info->projs[pn_Load_M], mem);
         }
@@ -575,6 +653,7 @@ static int optimize_load(ir_node *load)
           if (info->projs[pn_Load_res]) {
             set_Proj_pred(info->projs[pn_Load_res], pred);
             set_nodes_block(info->projs[pn_Load_res], get_nodes_block(pred));
+            pred_info->projs[pn_Load_res] = info->projs[pn_Load_res];
           }
           if (info->projs[pn_Load_M]) {
             /* Actually, this if should not be necessary.  Construct the Loads
@@ -584,29 +663,36 @@ static int optimize_load(ir_node *load)
         }
 
         /* no exception */
-        if (info->projs[pn_Load_X_except])
+        if (info->projs[pn_Load_X_except]) {
           exchange(info->projs[pn_Load_X_except], new_Bad());
+          res |= CF_CHANGED;
+        }
 
-        return 1;
+        return res |= DF_CHANGED;
       }
     }
 
     /* follow only Load chains */
     if (get_irn_op(pred) != op_Load)
       break;
-   }
+
+    /* check for cycles */
+    if (NODE_VISITED(pred_info))
+      break;
+    MARK_NODE(pred_info);
+  }
   return res;
 }
 
 /**
  * optimize a Store
  */
-static int optimize_store(ir_node *store)
+static unsigned optimize_store(ir_node *store)
 {
   ldst_info_t *info = get_irn_link(store);
   ir_node *pred, *mem, *ptr, *value, *block;
   ir_mode *mode;
-  int res = 0;
+  unsigned res = 0;
 
   if (get_Store_volatility(store) == volatility_is_volatile)
     return 0;
@@ -615,7 +701,7 @@ static int optimize_store(ir_node *store)
    * BEWARE: one might think that checking the modes is useless, because
    * if the pointers are identical, they refer to the same object.
    * This is only true in strong typed languages, not is C were the following
-   * is possible *(type1 *)p = a; *(type2 *)p = b ...
+   * is possible *(ir_type1 *)p = a; *(ir_type2 *)p = b ...
    */
 
   ptr   = get_Store_ptr(store);
@@ -631,7 +717,8 @@ static int optimize_store(ir_node *store)
   mode  = get_irn_mode(value);
 
   /* follow the memory chain as long as there are only Loads */
-  for (pred = skip_Proj(mem); ; pred = skip_Proj(get_Load_mem(pred))) {
+  INC_MASTER();
+  for (pred = skip_Proj(mem); pred != store; pred = skip_Proj(get_Load_mem(pred))) {
     ldst_info_t *pred_info = get_irn_link(pred);
 
     if (get_irn_op(pred) == op_Store && get_Store_ptr(pred) == ptr &&
@@ -645,7 +732,7 @@ static int optimize_store(ir_node *store)
       if (get_Store_volatility(pred) != volatility_is_volatile && !pred_info->projs[pn_Store_X_except]) {
         DBG_OPT_WAW(pred, store);
         exchange( pred_info->projs[pn_Store_M], get_Store_mem(pred) );
-        return 1;
+        return DF_CHANGED;
       }
     }
     else if (get_irn_op(pred) == op_Load && get_Load_ptr(pred) == ptr &&
@@ -657,35 +744,42 @@ static int optimize_store(ir_node *store)
       if (! info->projs[pn_Store_X_except]) {
         DBG_OPT_WAR(store, pred);
         exchange( info->projs[pn_Store_M], mem );
-        return 1;
+        return DF_CHANGED;
       }
     }
 
     /* follow only Load chains */
     if (get_irn_op(pred) != op_Load)
       break;
+
+    /* check for cycles */
+    if (NODE_VISITED(pred_info))
+      break;
+    MARK_NODE(pred_info);
   }
   return res;
 }
 
 /**
- * walker, optimizes Phi after Stores:
+ * walker, optimizes Phi after Stores to identical places:
  * Does the following optimization:
+ * @verbatim
  *
  *   val1   val2   val3          val1  val2  val3
  *    |      |      |               \    |    /
  *   Str    Str    Str               \   |   /
- *      \    |    /                     Phi
+ *      \    |    /                   PhiData
  *       \   |   /                       |
  *        \  |  /                       Str
- *          Phi
+ *          PhiM
  *
- * This removes the number of stores and allows for predicated execution.
- * Moves Stores back to the end of a function which may be bad
+ * @endverbatim
+ * This reduces the number of stores and allows for predicated execution.
+ * Moves Stores back to the end of a function which may be bad.
  *
- * Is only allowed if the predecessor blocks have only one successor.
+ * This is only possible if the predecessor blocks have only one successor.
  */
-static int optimize_phi(ir_node *phi, void *env)
+static unsigned optimize_phi(ir_node *phi, void *env)
 {
   walk_env_t *wenv = env;
   int i, n;
@@ -696,6 +790,7 @@ static int optimize_phi(ir_node *phi, void *env)
   dbg_info *db = NULL;
   ldst_info_t *info;
   block_info_t *bl_info;
+  unsigned res = 0;
 
   /* Must be a memory Phi */
   if (get_irn_mode(phi) != mode_M)
@@ -710,11 +805,11 @@ static int optimize_phi(ir_node *phi, void *env)
   if (get_irn_op(store) != op_Store)
     return 0;
 
-  /* abort on bad blocks */
-  if (is_Bad(get_nodes_block(store)))
+  /* abort on dead blocks */
+  if (is_Block_dead(get_nodes_block(store)))
     return 0;
 
-  /* check if the block has only one output */
+  /* check if the block has only one successor */
   bl_info = get_irn_link(get_nodes_block(store));
   if (bl_info->flags)
     return 0;
@@ -731,7 +826,7 @@ static int optimize_phi(ir_node *phi, void *env)
     if (get_irn_op(pred) != op_Store)
       return 0;
 
-    if (mode != get_irn_mode(get_Store_value(pred)) || ptr != get_Store_ptr(pred))
+    if (ptr != get_Store_ptr(pred) || mode != get_irn_mode(get_Store_value(pred)))
       return 0;
 
     info = get_irn_link(pred);
@@ -740,11 +835,11 @@ static int optimize_phi(ir_node *phi, void *env)
     if (exc != info->exc_block)
       return 0;
 
-    /* abort on bad blocks */
-    if (is_Bad(get_nodes_block(store)))
+    /* abort on dead blocks */
+    if (is_Block_dead(get_nodes_block(store)))
       return 0;
 
-    /* check if the block has only one output */
+    /* check if the block has only one successor */
     bl_info = get_irn_link(get_nodes_block(store));
     if (bl_info->flags)
       return 0;
@@ -752,17 +847,17 @@ static int optimize_phi(ir_node *phi, void *env)
 
   /*
    * ok, when we are here, we found all predecessors of a Phi that
-   * are Stores to the same address. That means whatever we do before
-   * we enter the block of the Phi, we do a Store.
-   * So, we can move the store to the current block:
+   * are Stores to the same address and size. That means whatever
+   * we do before we enter the block of the Phi, we do a Store.
+   * So, we can move the Store to the current block:
    *
    *   val1    val2    val3          val1  val2  val3
    *    |       |       |               \    |    /
    * | Str | | Str | | Str |             \   |   /
-   *      \     |     /                     Phi
+   *      \     |     /                   PhiData
    *       \    |    /                       |
    *        \   |   /                       Str
-   *           Phi
+   *           PhiM
    *
    * Is only allowed if the predecessor blocks have only one successor.
    */
@@ -814,16 +909,18 @@ static int optimize_phi(ir_node *phi, void *env)
     if (n > 1) {
       /* the exception block should be optimized as some inputs are identical now */
     }
+
+    res |= CF_CHANGED;
   }
 
-  /* sixt step: replace old Phi */
+  /* sixth step: replace old Phi */
   exchange(phi, projM);
 
-  return 1;
+  return res | DF_CHANGED;
 }
 
 /**
- * walker, collects all Load/Store/Proj nodes
+ * walker, do the optimizations
  */
 static void do_load_store_optimize(ir_node *n, void *env)
 {
@@ -855,15 +952,18 @@ void optimize_load_store(ir_graph *irg)
   walk_env_t env;
 
   assert(get_irg_phase_state(irg) != phase_building);
+  assert(get_irg_pinned(irg) != op_pin_state_floats &&
+    "LoadStore optimization needs pinned graph");
 
-  if (!get_opt_redundant_LoadStore())
+  if (! get_opt_redundant_loadstore())
     return;
 
   obstack_init(&env.obst);
   env.changes = 0;
 
   /* init the links, then collect Loads/Stores/Proj's in lists */
-  irg_walk_graph(irg, init_links, collect_nodes, &env);
+  master_visited = 0;
+  irg_walk_graph(irg, firm_clear_link, collect_nodes, &env);
 
   /* now we have collected enough information, optimize */
   irg_walk_graph(irg, NULL, do_load_store_optimize, &env);
@@ -874,9 +974,10 @@ void optimize_load_store(ir_graph *irg)
   if (env.changes) {
     if (get_irg_outs_state(current_ir_graph) == outs_consistent)
       set_irg_outs_inconsistent(current_ir_graph);
+  }
 
-    /* is this really needed: Yes, as exception block may get bad but this might be tested */
-    if (get_irg_dom_state(current_ir_graph) == dom_consistent)
-      set_irg_dom_inconsistent(current_ir_graph);
+  if (env.changes & CF_CHANGED) {
+    /* is this really needed: Yes, control flow changed, block might get Bad. */
+    set_irg_doms_inconsistent(current_ir_graph);
   }
 }