Re: [PR68083] stop ifcombine from moving uninitialized uses before their guards

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Oct 30, 2015 at 7:02 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Oct 30, 2015, Alexandre Oliva <aoliva@redhat.com> wrote:
>
>> Incidentally, bb_no_side_effects_p (inner_cond_bb) is called in all four
>> tests in tree_ssa_ifcombine_bb_1, for each outer_cond_bb that
>> tree_ssa_ifcombine_bb might choose.  Is there any reason to not factor
>> it out to the test that checks whether the inner_cond_bb is indeed an
>> if_then_else block, early in tree_ssa_ifcombine_bb, so as to
>> short-circuit the whole thing when the inner block is not viable?
>
> Like this...
>
> Bail out early if the inner block has side effects or is otherwise not
> eligible for ifcombine.
>
> Regstrapped on x86_64-linux-gnu.  Ok to install?

Ok.

Thanks,
Richard.

>
> for  gcc/ChangeLog
>
>         * tree-ssa-ifcombine.c (tree_ssa_ifcombine_bb_1): Factor out
>         bb_no_side_effects_p tests...
>         (tree_ssa_ifcombine_bb): ... here.
> ---
>  gcc/tree-ssa-ifcombine.c |   15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/tree-ssa-ifcombine.c b/gcc/tree-ssa-ifcombine.c
> index 622dc6b..3b60968 100644
> --- a/gcc/tree-ssa-ifcombine.c
> +++ b/gcc/tree-ssa-ifcombine.c
> @@ -579,8 +579,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
>       the inner cond_bb having no side-effects.  */
>    if (phi_pred_bb != else_bb
>        && recognize_if_then_else (outer_cond_bb, &inner_cond_bb, &else_bb)
> -      && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
> -      && bb_no_side_effects_p (inner_cond_bb))
> +      && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb))
>      {
>        /* We have
>            <outer_cond_bb>
> @@ -598,8 +597,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
>    /* And a version where the outer condition is negated.  */
>    if (phi_pred_bb != else_bb
>        && recognize_if_then_else (outer_cond_bb, &else_bb, &inner_cond_bb)
> -      && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
> -      && bb_no_side_effects_p (inner_cond_bb))
> +      && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb))
>      {
>        /* We have
>            <outer_cond_bb>
> @@ -620,8 +618,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
>       having no side-effects.  */
>    if (phi_pred_bb != then_bb
>        && recognize_if_then_else (outer_cond_bb, &then_bb, &inner_cond_bb)
> -      && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
> -      && bb_no_side_effects_p (inner_cond_bb))
> +      && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb))
>      {
>        /* We have
>            <outer_cond_bb>
> @@ -638,8 +635,7 @@ tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block outer_cond_bb,
>    /* And a version where the outer condition is negated.  */
>    if (phi_pred_bb != then_bb
>        && recognize_if_then_else (outer_cond_bb, &inner_cond_bb, &then_bb)
> -      && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
> -      && bb_no_side_effects_p (inner_cond_bb))
> +      && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb))
>      {
>        /* We have
>            <outer_cond_bb>
> @@ -676,7 +672,8 @@ tree_ssa_ifcombine_bb (basic_block inner_cond_bb)
>         if (a && b)
>          ;
>       This requires a single predecessor of the inner cond_bb.  */
> -  if (single_pred_p (inner_cond_bb))
> +  if (single_pred_p (inner_cond_bb)
> +      && bb_no_side_effects_p (inner_cond_bb))
>      {
>        basic_block outer_cond_bb = single_pred (inner_cond_bb);
>
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer