BugFix: a var hides another
[libfirm] / ir / opt / ldstopt.c
index d583948..5733efe 100644 (file)
 
 #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;
 
 /**
@@ -71,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;
 
 /**
@@ -78,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 */
 };
 
 /**
@@ -88,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
@@ -133,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);
 
@@ -142,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;
@@ -151,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");
 
@@ -168,12 +177,12 @@ static int update_exc(ldst_info_t *info, ir_node *block, int pos)
 /**
  * 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;
 
@@ -195,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);
@@ -207,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;
 
@@ -220,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);
@@ -362,13 +397,13 @@ static compound_graph_path *get_accessed_path(ir_node *ptr) {
 /**
  * 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)
@@ -401,7 +436,7 @@ static int optimize_load(ir_node *load)
 
           exchange(info->projs[pn_Load_X_except], new_Bad());
           info->projs[pn_Load_X_except] = NULL;
-          res = 1;
+          res |= CF_CHANGED;
         }
       }
     }
@@ -414,7 +449,7 @@ static int optimize_load(ir_node *load)
        */
        exchange(info->projs[pn_Load_X_except], new_Bad());
        info->projs[pn_Load_X_except] = NULL;
-       res = 1;
+       res |= CF_CHANGED;
     }
   }
 
@@ -425,7 +460,7 @@ static int optimize_load(ir_node *load)
     /* a Load which value is neither used nor exception checked, remove it */
     exchange(info->projs[pn_Load_M], mem);
 
-    return 1;
+    return res | DF_CHANGED;
   }
 
   /* Load from a constant polymorphic field, where we can resolve
@@ -442,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 */
@@ -457,7 +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 = 1;
+        res |= CF_CHANGED;
       }
 
       if (variability_constant == get_entity_variability(ent)
@@ -473,7 +508,7 @@ static int optimize_load(ir_node *load)
         /* no memory */
         if (info->projs[pn_Load_M]) {
           exchange(info->projs[pn_Load_M], mem);
-          res = 1;
+          res |= DF_CHANGED;
         }
 
         /* no result :-) */
@@ -483,7 +518,7 @@ static int optimize_load(ir_node *load)
 
             DBG_OPT_RC(load, c);
             exchange(info->projs[pn_Load_res], c);
-            return 1;
+            return DF_CHANGED | res;
           }
         }
       }
@@ -514,11 +549,11 @@ static int optimize_load(ir_node *load)
 
           if (info->projs[pn_Load_M]) {
             exchange(info->projs[pn_Load_M], mem);
-            res = 1;
+            res |= DF_CHANGED;
           }
           if (info->projs[pn_Load_res]) {
             exchange(info->projs[pn_Load_res], copy_const_value(c));
-            return 1;
+            return res | DF_CHANGED;
           }
         }
         else {
@@ -540,10 +575,17 @@ static int optimize_load(ir_node *load)
   if (get_irn_out_n(ptr) <= 1)
     return res;
 
-  /* follow the memory chain as long as there are only Loads
-   * and try to replace current Load or Store by a previous one
+  /*
+   * 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.
    */
-  for (pred = skip_Proj(mem); ; pred = skip_Proj(get_Load_mem(pred))) {
+  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.
@@ -553,8 +595,6 @@ static int optimize_load(ir_node *load)
 
     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
@@ -576,13 +616,15 @@ static int optimize_load(ir_node *load)
           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());
+          res |= CF_CHANGED;
+        }
 
         if (info->projs[pn_Load_res])
           exchange(info->projs[pn_Load_res], value);
 
-        return 1;
+        return res | DF_CHANGED;
       }
     }
     else if (get_irn_op(pred) == op_Load && get_Load_ptr(pred) == ptr &&
@@ -597,8 +639,6 @@ static int optimize_load(ir_node *load)
        * 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]) {
@@ -623,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;
@@ -670,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 &&
@@ -684,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 &&
@@ -696,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;
@@ -735,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)
@@ -749,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;
@@ -770,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);
@@ -779,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;
@@ -791,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.
    */
@@ -853,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 optiimizations
  */
 static void do_load_store_optimize(ir_node *n, void *env)
 {
@@ -904,7 +962,8 @@ void optimize_load_store(ir_graph *irg)
   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);
@@ -915,8 +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 (env.changes & CF_CHANGED) {
+    /* is this really needed: Yes, control flow changed, block might get Bad. */
     if (get_irg_dom_state(current_ir_graph) == dom_consistent)
       set_irg_dom_inconsistent(current_ir_graph);
   }