Message ID | 20160503181618.GE26501@tucnak.zalov.cz |
---|---|
State | New |
Headers | show |
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
--- 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;