2009-07-11 Michael Barker <mike@middlesoft.co.uk>
[mono.git] / mono / mini / branch-opts.c
index 5e11f443784be7a9dc0a6309223326e6b540ad3a..3d27d244404c66266be048f9c53d9c1dc5739c8e 100644 (file)
 
 #ifndef DISABLE_JIT
  
- /*
+
+/*
+ * Returns true if @bb is a basic block which falls through the next block.
+ * TODO verify if it helps to check if the bb last ins is a branch to its successor. 
+ */
+static gboolean
+mono_bb_is_fall_through (MonoCompile *cfg, MonoBasicBlock *bb)
+{
+       return  bb->next_bb && bb->next_bb->region == bb->region && /*fall throught between regions is not really interesting or useful*/
+                       (bb->last_ins == NULL || !MONO_IS_BRANCH_OP (bb->last_ins)); /*and the last op can't be a branch too*/
+}
+
+/*
  * Used by the arch code to replace the exception handling
  * with a direct branch. This is safe to do if the 
  * exception object isn't used, no rethrow statement and
@@ -296,20 +308,37 @@ mono_if_conversion (MonoCompile *cfg)
                bb2 = bb->out_bb [1];
 
                if (bb1->in_count == 1 && bb2->in_count == 1 && bb1->out_count == 1 && bb2->out_count == 1 && bb1->out_bb [0] == bb2->out_bb [0]) {
-                       MonoInst *prev, *compare, *branch, *ins1, *ins2, *cmov, *move, *tmp;
+                       MonoInst *compare, *branch, *ins1, *ins2, *cmov, *move, *tmp;
+                       MonoBasicBlock *true_bb, *false_bb;
                        gboolean simple, ret;
                        int dreg, tmp_reg;
                        CompType comp_type;
 
+                       if (bb->last_ins && (bb->last_ins->opcode == OP_BR_REG || bb->last_ins->opcode == OP_BR))
+                               continue;
+
+                       /* Find the compare instruction */
+                       if (!bb->last_ins || !bb->last_ins->prev)
+                               continue;
+                       branch = bb->last_ins;
+                       compare = branch->prev;
+
+                       if (!MONO_IS_COND_BRANCH_OP (branch))
+                               /* This can happen if a cond branch is optimized away */
+                               continue;
+
+                       true_bb = branch->inst_true_bb;
+                       false_bb = branch->inst_false_bb;
+
                        /* 
                         * Check that bb1 and bb2 are 'simple' and both assign to the same
                         * variable.
                         */
                        /* FIXME: Get rid of the nops earlier */
-                       ins1 = bb1->code;
+                       ins1 = true_bb->code;
                        while (ins1 && ins1->opcode == OP_NOP)
                                ins1 = ins1->next;
-                       ins2 = bb2->code;
+                       ins2 = false_bb->code;
                        while (ins2 && ins2->opcode == OP_NOP)
                                ins2 = ins2->next;
                        if (!(ins1 && ins2 && ins1->dreg == ins2->dreg && ins1->dreg != -1))
@@ -331,21 +360,6 @@ mono_if_conversion (MonoCompile *cfg)
                        if (!(MONO_INS_HAS_NO_SIDE_EFFECT (ins1) && MONO_INS_HAS_NO_SIDE_EFFECT (ins2)))
                                continue;
 
-                       if (bb->last_ins && (bb->last_ins->opcode == OP_BR_REG || bb->last_ins->opcode == OP_BR))
-                               continue;
-
-                       /* Find the compare instruction */
-                       /* FIXME: Optimize this using prev */
-                       prev = NULL;
-                       compare = bb->code;
-                       g_assert (compare);
-                       while (compare->next && !MONO_IS_COND_BRANCH_OP (compare->next)) {
-                               prev = compare;
-                               compare = compare->next;
-                       }
-                       g_assert (compare->next && MONO_IS_COND_BRANCH_OP (compare->next));
-                       branch = compare->next;
-
                        /* Moving ins1/ins2 could change the comparison */
                        /* FIXME: */
                        if (!((compare->sreg1 != ins1->dreg) && (compare->sreg2 != ins1->dreg)))
@@ -391,8 +405,8 @@ mono_if_conversion (MonoCompile *cfg)
                        }
 
                        /* Remove ins1/ins2 from bb1/bb2 */
-                       MONO_REMOVE_INS (bb1, ins1);
-                       MONO_REMOVE_INS (bb2, ins2);
+                       MONO_REMOVE_INS (true_bb, ins1);
+                       MONO_REMOVE_INS (false_bb, ins2);
 
                        /* Move ins1 and ins2 before the comparison */
                        /* ins1 comes first to avoid ins1 overwriting an argument of ins2 */
@@ -426,16 +440,16 @@ mono_if_conversion (MonoCompile *cfg)
 
                        /* Rewrite the branch */
                        branch->opcode = OP_BR;
-                       branch->inst_target_bb = bb1->out_bb [0];
+                       branch->inst_target_bb = true_bb->out_bb [0];
                        mono_link_bblock (cfg, bb, branch->inst_target_bb);
 
                        /* Reorder bblocks */
-                       mono_unlink_bblock (cfg, bb, bb1);
-                       mono_unlink_bblock (cfg, bb, bb2);
-                       mono_unlink_bblock (cfg, bb1, bb1->out_bb [0]);
-                       mono_unlink_bblock (cfg, bb2, bb2->out_bb [0]);
-                       mono_remove_bblock (cfg, bb1);
-                       mono_remove_bblock (cfg, bb2);
+                       mono_unlink_bblock (cfg, bb, true_bb);
+                       mono_unlink_bblock (cfg, bb, false_bb);
+                       mono_unlink_bblock (cfg, true_bb, true_bb->out_bb [0]);
+                       mono_unlink_bblock (cfg, false_bb, false_bb->out_bb [0]);
+                       mono_remove_bblock (cfg, true_bb);
+                       mono_remove_bblock (cfg, false_bb);
 
                        /* Merge bb and its successor if possible */
                        if ((bb->out_bb [0]->in_count == 1) && (bb->out_bb [0] != cfg->bb_exit) &&
@@ -456,7 +470,7 @@ mono_if_conversion (MonoCompile *cfg)
 
                if ((bb2->in_count == 1 && bb2->out_count == 1 && bb2->out_bb [0] == bb1) ||
                        (bb1->in_count == 1 && bb1->out_count == 1 && bb1->out_bb [0] == bb2)) {
-                       MonoInst *prev, *compare, *branch, *ins1, *cmov, *tmp;
+                       MonoInst *compare, *branch, *ins1, *cmov, *tmp;
                        gboolean simple;
                        int dreg, tmp_reg;
                        CompType comp_type;
@@ -494,16 +508,15 @@ mono_if_conversion (MonoCompile *cfg)
                                continue;
 
                        /* Find the compare instruction */
-                       /* FIXME: Optimize this using prev */
-                       prev = NULL;
-                       compare = bb->code;
-                       g_assert (compare);
-                       while (compare->next && !MONO_IS_COND_BRANCH_OP (compare->next)) {
-                               prev = compare;
-                               compare = compare->next;
-                       }
-                       g_assert (compare->next && MONO_IS_COND_BRANCH_OP (compare->next));
-                       branch = compare->next;
+
+                       if (!bb->last_ins || !bb->last_ins->prev)
+                               continue;
+                       branch = bb->last_ins;
+                       compare = branch->prev;
+
+                       if (!MONO_IS_COND_BRANCH_OP (branch))
+                               /* This can happen if a cond branch is optimized away */
+                               continue;
 
                        /* FIXME: */
                        comp_type = mono_opcode_to_type (branch->opcode, compare->opcode);
@@ -581,6 +594,18 @@ mono_if_conversion (MonoCompile *cfg)
                        if ((bb->out_bb [0]->in_count == 1) && (bb->out_bb [0] != cfg->bb_exit) &&
                                (bb->region == bb->out_bb [0]->region)) {
                                mono_merge_basic_blocks (cfg, bb, bb->out_bb [0]);
+
+                               /* 
+                                * bbn might have fallen through to the next bb without a branch, 
+                                * have to add one now (#474718).
+                                * FIXME: Maybe need to do this more generally in 
+                                * merge_basic_blocks () ?
+                                */
+                               if (!(bb->last_ins && MONO_IS_BRANCH_OP (bb->last_ins)) && bb->out_count) {
+                                       MONO_INST_NEW (cfg, ins1, OP_BR);
+                                       ins1->inst_target_bb = bb->out_bb [0];
+                                       MONO_ADD_INS (bb, ins1);
+                               }
                                goto restart;
                        }
                }
@@ -894,13 +919,7 @@ remove_block_if_useless (MonoCompile *cfg, MonoBasicBlock *bb, MonoBasicBlock *p
                }
                
                mono_unlink_bblock (cfg, bb, target_bb);
-               
-               if ((previous_bb != cfg->bb_entry) &&
-                               (previous_bb->region == bb->region) &&
-                               ((previous_bb->last_ins == NULL) ||
-                               ((previous_bb->last_ins->opcode != OP_BR) &&
-                               (! (MONO_IS_COND_BRANCH_OP (previous_bb->last_ins))) &&
-                               (previous_bb->last_ins->opcode != OP_SWITCH)))) {
+               if (previous_bb != cfg->bb_entry && mono_bb_is_fall_through (cfg, previous_bb)) {
                        for (i = 0; i < previous_bb->out_count; i++) {
                                if (previous_bb->out_bb [i] == target_bb) {
                                        MonoInst *jump;
@@ -939,21 +958,27 @@ mono_merge_basic_blocks (MonoCompile *cfg, MonoBasicBlock *bb, MonoBasicBlock *b
                mono_unlink_bblock (cfg, bbn, bbn->out_bb [0]);
 
        /* Handle the branch at the end of the bb */
-       for (inst = bb->code; inst != NULL; inst = inst->next) {
-               if (inst->opcode == OP_CALL_HANDLER) {
-                       g_assert (inst->inst_target_bb == bbn);
-                       NULLIFY_INS (inst);
+       if (bb->has_call_handler) {
+               for (inst = bb->code; inst != NULL; inst = inst->next) {
+                       if (inst->opcode == OP_CALL_HANDLER) {
+                               g_assert (inst->inst_target_bb == bbn);
+                               NULLIFY_INS (inst);
+                       }
                }
-               if (MONO_IS_JUMP_TABLE (inst)) {
-                       int i;
-                       MonoJumpInfoBBTable *table = MONO_JUMP_TABLE_FROM_INS (inst);
-                       for (i = 0; i < table->table_size; i++ ) {
-                               /* Might be already NULL from a previous merge */
-                               if (table->table [i])
-                                       g_assert (table->table [i] == bbn);
-                               table->table [i] = NULL;
+       }
+       if (bb->has_jump_table) {
+               for (inst = bb->code; inst != NULL; inst = inst->next) {
+                       if (MONO_IS_JUMP_TABLE (inst)) {
+                               int i;
+                               MonoJumpInfoBBTable *table = MONO_JUMP_TABLE_FROM_INS (inst);
+                               for (i = 0; i < table->table_size; i++ ) {
+                                       /* Might be already NULL from a previous merge */
+                                       if (table->table [i])
+                                               g_assert (table->table [i] == bbn);
+                                       table->table [i] = NULL;
+                               }
+                               /* Can't nullify this as later instructions depend on it */
                        }
-                       /* Can't nullify this as later instructions depend on it */
                }
        }
        if (bb->last_ins && MONO_IS_COND_BRANCH_OP (bb->last_ins)) {
@@ -964,6 +989,9 @@ mono_merge_basic_blocks (MonoCompile *cfg, MonoBasicBlock *bb, MonoBasicBlock *b
                NULLIFY_INS (bb->last_ins);
        }
 
+       bb->has_call_handler |= bbn->has_call_handler;
+       bb->has_jump_table |= bbn->has_jump_table;
+
        if (bb->last_ins) {
                if (bbn->code) {
                        bb->last_ins->next = bbn->code;
@@ -974,6 +1002,7 @@ mono_merge_basic_blocks (MonoCompile *cfg, MonoBasicBlock *bb, MonoBasicBlock *b
                bb->code = bbn->code;
                bb->last_ins = bbn->last_ins;
        }
+
        for (prev_bb = cfg->bb_entry; prev_bb && prev_bb->next_bb != bbn; prev_bb = prev_bb->next_bb)
                ;
        if (prev_bb) {
@@ -984,6 +1013,16 @@ mono_merge_basic_blocks (MonoCompile *cfg, MonoBasicBlock *bb, MonoBasicBlock *b
                        bb->next_bb = bbn->next_bb;
        }
        mono_nullify_basic_block (bbn);
+
+       /* 
+        * If bbn fell through to its next bblock, have to add a branch, since bb
+        * will not fall though to the same bblock (#513931).
+        */
+       if (bb->last_ins && bb->out_count == 1 && bb->out_bb [0] != bb->next_bb && !MONO_IS_BRANCH_OP (bb->last_ins)) {
+               MONO_INST_NEW (cfg, inst, OP_BR);
+               inst->inst_target_bb = bb->out_bb [0];
+               MONO_ADD_INS (bb, inst);
+       }
 }
 
 static void
@@ -1065,32 +1104,30 @@ mono_remove_critical_edges (MonoCompile *cfg)
                        int in_bb_index;
                        for (in_bb_index = 0; in_bb_index < bb->in_count; in_bb_index++) {
                                MonoBasicBlock *in_bb = bb->in_bb [in_bb_index];
-                               if (in_bb->out_count > 1) {
+                               /* 
+                                * Have to remove non-critical edges whose source ends with a BR_REG
+                                * ins too, since inserting a computation before the BR_REG could 
+                                * overwrite the sreg1 of the ins.
+                                */
+                               if ((in_bb->out_count > 1) || (in_bb->out_count == 1 && in_bb->last_ins && in_bb->last_ins->opcode == OP_BR_REG)) {
                                        MonoBasicBlock *new_bb = mono_mempool_alloc0 ((cfg)->mempool, sizeof (MonoBasicBlock));
                                        new_bb->block_num = cfg->num_bblocks++;
 //                                     new_bb->real_offset = bb->real_offset;
                                        new_bb->region = bb->region;
                                        
                                        /* Do not alter the CFG while altering the BB list */
-                                       if (previous_bb->region == bb->region) {
+                                       if (mono_bb_is_fall_through (cfg, previous_bb)) {
                                                if (previous_bb != cfg->bb_entry) {
-                                                       /* If previous_bb "followed through" to bb, */
-                                                       /* keep it linked with a OP_BR */
-                                                       if ((previous_bb->last_ins == NULL) ||
-                                                                       ((previous_bb->last_ins->opcode != OP_BR) &&
-                                                                       (! (MONO_IS_COND_BRANCH_OP (previous_bb->last_ins))) &&
-                                                                       (previous_bb->last_ins->opcode != OP_SWITCH))) {
-                                                               int i;
-                                                               /* Make sure previous_bb really falls through bb */
-                                                               for (i = 0; i < previous_bb->out_count; i++) {
-                                                                       if (previous_bb->out_bb [i] == bb) {
-                                                                               MonoInst *jump;
-                                                                               MONO_INST_NEW (cfg, jump, OP_BR);
-                                                                               MONO_ADD_INS (previous_bb, jump);
-                                                                               jump->cil_code = previous_bb->cil_code;
-                                                                               jump->inst_target_bb = bb;
-                                                                               break;
-                                                                       }
+                                                       int i;
+                                                       /* Make sure previous_bb really falls through bb */
+                                                       for (i = 0; i < previous_bb->out_count; i++) {
+                                                               if (previous_bb->out_bb [i] == bb) {
+                                                                       MonoInst *jump;
+                                                                       MONO_INST_NEW (cfg, jump, OP_BR);
+                                                                       MONO_ADD_INS (previous_bb, jump);
+                                                                       jump->cil_code = previous_bb->cil_code;
+                                                                       jump->inst_target_bb = bb;
+                                                                       break;
                                                                }
                                                        }
                                                } else {