[PATH]: PR middlend/38385 fix

Richard Guenther richard.guenther@gmail.com
Thu Jan 8 12:24:00 GMT 2009


On Thu, Jan 8, 2009 at 12:29 PM, Tomas Bily <tbily@suse.cz> wrote:
> Hi,
>
>  as Richard recommended this patch fixes pr38385 by propagation and
> removing of phis from loop exit block.
>
> Tested and bootstraped on x86_64-unknown-linux-gnu.
> Ok for mainline ?
>
>> >>>  This pr is caused because after removing bbs in generate_builtin
>> >>> (tree-loop-distribution.c) some phis has bad incoming edges. This
>> >>> patch tryes to fix it.
>> >>>
>> >>> Tested and bootstraped on x86_64-unknown-linux-gnu.
>> >>> Ok for mainline ?
>> >>>
>> >>
>> >> Looks good to me, but I cannot approve it.
>> >
>> > I don't follow.  You create an edge from the preheader to the loop exit
>> > and now use the values from the exit edge for that edge?  If we
>> > are in loop-closed SSA form we should not need to care at all here.
>> > It is more likely that we need to split some block before removing the
>> > loop.
>>
>> Err, wait.  These _are_ the loop-closed PHI nodes.  You should be able
>> to just remove them by propagation.
>>
>> Richard.
>
> Greetings
> Tomas
>
> Changelog:
>
>        * tree-loop-distribution.c (prop_phis): New function.
>        (generate_builtin): Call prop_phis.
>        * testsuite/gcc.dg/tree-ssa/pr38385.c: New file.
>
>
> Index: tree-loop-distribution.c
> ===================================================================
> --- tree-loop-distribution.c    (revision 143184)
> +++ tree-loop-distribution.c    (working copy)
> @@ -331,6 +331,37 @@ generate_memset_zero (gimple stmt, tree
>   return res;
>  }
>
> +/* Propagate phis in bb b to their uses and remove them.  */

capital B.

> +static void
> +prop_phis (basic_block b)
> +{
> +  gimple_stmt_iterator psi;
> +  gimple_seq phis = phi_nodes (b);
> +
> +  for (psi = gsi_start (phis); !gsi_end_p (psi); )
> +    {
> +      gimple phi = gsi_stmt (psi);
> +      tree def = gimple_phi_result (phi), use = gimple_phi_arg_def (phi, 0);

Please add an gcc_assert (gimple_phi_num_args (phi) == 1) here.

> +      if (!is_gimple_reg (def))
> +       {
> +         imm_use_iterator iter;
> +         use_operand_p use_p;
> +         gimple stmt;
> +
> +         FOR_EACH_IMM_USE_STMT (stmt, iter, def)
> +           FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> +             SET_USE (use_p, use);
> +       }
> +      else
> +        replace_uses_by (def, use);
> +
> +      remove_phi_node (&psi, true);
> +    }
> +}
> +
> +
>  /* Tries to generate a builtin function for the instructions of LOOP
>    pointed to by the bits set in PARTITION.  Returns true when the
>    operation succeeded.  */
> @@ -400,12 +431,15 @@ generate_builtin (struct loop *loop, bit
>       unsigned nbbs = loop->num_nodes;
>       basic_block src = loop_preheader_edge (loop)->src;
>       basic_block dest = single_exit (loop)->dest;
> +      prop_phis (dest);
>       make_edge (src, dest, EDGE_FALLTHRU);
>       set_immediate_dominator (CDI_DOMINATORS, dest, src);
>       cancel_loop_tree (loop);
>
>       for (i = 0; i < nbbs; i++)
>        delete_basic_block (bbs[i]);
> +
> +      merge_blocks (src, dest);

Is this for optimization only or necessary?  If for optimization please
defer this to the next cfg_cleanup run.

Ok with these changes.

Thanks,
Richard.

>     }
>
>  end:
>



More information about the Gcc-patches mailing list