PR middle-end/44485 (error_mark instatement)

[Richard Guenther]

On Sat, Jun 12, 2010 at 12:54 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> after function is found noreturn, we have to remove its return value.  Use of the return
> value remains in the caller function body until next removal of unreachable code.  For this
> reason I added code replacing SSA_NAME with error_mark.
>
> This unfortunately triggers ICE when fixup_cfg decide to update on of the statements (this
> happens when the other statement is also call).
>
> THis patch just modify fixup_noreturn_call to remove those basic blocks proactively.
> It also seems to fix the Ada problem with the same symptom.
>
> Bootstrapped/regtested x86_64-linux, seems to make sense?



> Honza
>
> static int
> foo (int si1, int si2)
> {
>  return si1 > 0 && si2 > 0 && si1 > -si2 || si1 < 0 && si2 < 0
>    && si1 < -si2 ? : si1 + si2;
> }
>
> struct S0
> {
>  unsigned short f1;
> };
> int g_4;
> struct S0 g_54 = {
>  3428
> };
>
> int
> func_21 (int * p_22, int * const int32p_24, unsigned p_25,
>         const int * p_26);
>
> void int324 (unsigned p_15, int * p_16, int * p_17, int * p_18)
> {
>  if (foo (g_4, func_21 (p_18, &g_4, 0, 0)))
>    {
>      for (g_54.f1; g_54.f1; g_54.f1 += 1)
>        {
>        }
>    }
> }
>
> int
> func_21 (int * p_22, int * const int32p_24, unsigned p_25,
>         const int * p_26)
> {
>  for (0; 1; p_25 += 1)
>  lbl_29:if (p_25)
>      goto lbl_28;
> lbl_28:for (p_25 = 0; p_25 < 9; p_25 += 1)
>    if (p_25)
>      goto lbl_29;
>  unsigned short l_53;
>  for (0; l_53; l_53 = foo)
>    {
>    }
>  return 0;
> }
>
>        PR middle-end/44485
>        * tree-cfgcleanup.c (fixup_noreturn_call): Remove basic block contianing
>        use of return value of function found to be noreturn.
> Index: tree-cfgcleanup.c
> ===================================================================
> --- tree-cfgcleanup.c   (revision 160614)
> +++ tree-cfgcleanup.c   (working copy)
> @@ -559,18 +559,35 @@ fixup_noreturn_call (gimple stmt)
>     {
>       tree op = gimple_call_lhs (stmt);
>       gimple_call_set_lhs (stmt, NULL_TREE);
> +
>       /* We need to remove SSA name to avoid checking.

To avoid checking errors?

>         All uses are dominated by the noreturn and thus will
> -        be removed afterwards.  */
> +        be removed afterwards.
> +        We proactively remove affected non-PHI statements to avoid
> +        fixup_cfg from trying to update them and crashing.  */
>       if (TREE_CODE (op) == SSA_NAME)
>        {
>          use_operand_p use_p;
>           imm_use_iterator iter;
>          gimple use_stmt;
> +         bitmap_iterator bi;
> +         unsigned int bb_index;
> +
> +         bitmap_head blocks;
> +         bitmap_initialize (&blocks, &bitmap_default_obstack);

I know you like to replace all bitmaps by bitmap_head, but in this
case please use a bitmap and the usual BITMAP_ALLOC/FREE.

Ok with that change.

Thanks,
Richard.

>           FOR_EACH_IMM_USE_STMT (use_stmt, iter, op)
> -           FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> -             SET_USE (use_p, error_mark_node);
> +           {
> +             if (gimple_code (use_stmt) != GIMPLE_PHI)
> +               bitmap_set_bit (&blocks, gimple_bb (use_stmt)->index);
> +             else
> +               FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> +                 SET_USE (use_p, error_mark_node);
> +           }
> +         EXECUTE_IF_SET_IN_BITMAP (&blocks, 0, bb_index, bi)
> +           delete_basic_block (BASIC_BLOCK (bb_index));
> +         bitmap_clear (&blocks);
> +         release_ssa_name (op);
>        }
>       update_stmt (stmt);
>       changed = true;
>