Re: [PATCH] Fix if-conversion (PR tree-optimization/70916)

[Richard Biener <rguenther at suse dot de>]

On May 3, 2016 8:16:18 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>As mentioned in the PR and on IRC, the initial problem on the testcase
>from this PR is that if-conv mistakenly assumes that some bb predicates
>are true predicates, when they really should not be.
>The problem is if we have a loop (perhaps finite) inside of some
>infinite
>loop, when computing post dominators we pick quite randomly some bb and
>add a fake edge to exit to that for the computation.  In the testcase
>we
>had in the inner loop:
>bb7:
>...
>if (something)
>{
>  bb14:
>    ...
>}
>bb8:;
>and we were unfortunate enough that we pretended to add fake edge to
>exit
>to exactly bb14.  Immediate dominator of bb14 is (correctly) bb7, but
>due to the fake edge to exit immediate post-dominator of bb7 used to be
>bb14, rather than bb8, so we thought that bb14 is a join block and used
>true predicate for it, when it should use something instead.
>
>The following patch fixes this by making the fake edges (still quite
>randomly chosen) explicit, so if we are unlucky enough we just don't
>if-convert, but don't miscompile stuff.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

>2016-05-03  Jakub Jelinek  <jakub@redhat.com>
>	    Richard Biener  <rguenther@suse.de>
>
>	PR tree-optimization/70916
>	* tree-if-conv.c: Include cfganal.h.
>	(pass_if_conversion::execute): Call connect_infinite_loops_to_exit
>	and remove_fake_exit_edges around the optimization pass.
>
>--- gcc/tree-if-conv.c.jj	2016-05-03 13:42:48.000000000 +0200
>+++ gcc/tree-if-conv.c	2016-05-03 14:06:59.375341147 +0200
>@@ -113,6 +113,7 @@ along with GCC; see the file COPYING3.
> #include "varasm.h"
> #include "builtins.h"
> #include "params.h"
>+#include "cfganal.h"
> 
> /* Only handle PHIs with no more arguments unless we are asked to by
>    simd pragma.  */
>@@ -2812,6 +2813,14 @@ pass_if_conversion::execute (function *f
>   if (number_of_loops (fun) <= 1)
>     return 0;
> 
>+  /* If there are infinite loops, during CDI_POST_DOMINATORS
>computation
>+     we can pick pretty much random bb inside of the infinite loop
>that
>+     has the fake edge.  If we are unlucky enough, this can confuse
>the
>+     add_to_predicate_list post-dominator check to optimize as if that
>+     bb or some other one is a join block when it actually is not.
>+     See PR70916.  */
>+  connect_infinite_loops_to_exit ();
>+
>   FOR_EACH_LOOP (loop, 0)
>     if (flag_tree_loop_if_convert == 1
> 	|| flag_tree_loop_if_convert_stores == 1
>@@ -2819,6 +2828,8 @@ pass_if_conversion::execute (function *f
> 	    && !loop->dont_vectorize))
>       todo |= tree_if_conversion (loop);
> 
>+  remove_fake_exit_edges ();
>+
>   if (flag_checking)
>     {
>       basic_block bb;
>
>	Jakub