Hi, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77848 identifies a situation where if-conversion causes degradation when the if-converted loop is not subsequently vectorized. The if-conversion pass does not have a cost model to avoid such degradations. However, it does have a capability to version the if-converted loop, so that the vectorizer can choose the if-converted version if vectorization occurs, or the unmodified version if vectorization does not occur. Currently versioning is only done under special circumstances.
This patch does two things: It requires loop versioning whenever loop vectorization is enabled so that such degradations can't occur; and it extends loop versioning to outer loops when such loops are of the right form for outer loop vectorization. The latter is needed to avoid introducing degradations with versioning of inner loops, which disturbs the pattern that outer loop vectorization expects. This is an embarrassingly simple patch, given how much time I spent going down other paths. The most surprising thing is that versioning the outer loop doesn't require any additional handshaking with the vectorizer. It just works. I've verified this on some examples, and we end up with the correct vectorization and with the unused loop nest discarded. The one remaining problem with this bug is that it precludes SLP from seeing if-converted loops to work on. With this patch, if the vectorizer can't vectorize an if-converted loop, the original version survives. We have one test case that fails when that happens, because it expected to do SLP vectorization on the if-converted statements: > FAIL: gcc.dg/vect/bb-slp-cond-1.c -flto -ffat-lto-objects > scan-tree-dump-times slp1 "basic block vectorized" 1 > FAIL: gcc.dg/vect/bb-slp-cond-1.c scan-tree-dump-times slp1 "basic block > vectorized" 1 Arguably, this shows a deficiency in SLP vectorization, since it won't see if-converted statements in non-loop code in any event. Eventually SLP should learn to handle these kinds of PHI statements itself. Bootstrapped and tested on powerpc64le-unknown-linux-gnu, with only the specified regression. Is this ok for trunk? Thanks, Bill [gcc] 2016-11-15 Bill Schmidt <wschm...@linux.vnet.ibm.com> PR tree-optimization/77848 * tree-if-conv.c (version_loop_for_if_conversion): When versioning an outer loop, only save basic block aux information for the inner loop. (versionable_outer_loop_p): New function. (tree_if_conversion): Always version a loop when vectorization is enabled; version the outer loop instead of the inner one if the pattern will be recognized for outer-loop vectorization. [gcc/testsuite] 2016-11-15 Bill Schmidt <wschm...@linux.vnet.ibm.com> PR tree-optimization/77848 * gfortran.dg/vect/pr78848.f: New test. Index: gcc/testsuite/gfortran.dg/vect/pr77848.f =================================================================== --- gcc/testsuite/gfortran.dg/vect/pr77848.f (revision 0) +++ gcc/testsuite/gfortran.dg/vect/pr77848.f (working copy) @@ -0,0 +1,24 @@ +! PR 77848: Verify versioning is on when vectorization fails +! { dg-do compile } +! { dg-options "-O3 -ffast-math -fdump-tree-ifcvt -fdump-tree-vect-details" } + + subroutine sub(x,a,n,m) + implicit none + real*8 x(*),a(*),atemp + integer i,j,k,m,n + real*8 s,t,u,v + do j=1,m + atemp=0.d0 + do i=1,n + if (abs(a(i)).gt.atemp) then + atemp=a(i) + k = i + end if + enddo + call dummy(atemp,k) + enddo + return + end + +! { dg-final { scan-tree-dump "LOOP_VECTORIZED" "ifcvt" } } +! { dg-final { scan-tree-dump "vectorized 0 loops in function" "vect" } } Index: gcc/tree-if-conv.c =================================================================== --- gcc/tree-if-conv.c (revision 242412) +++ gcc/tree-if-conv.c (working copy) @@ -2533,6 +2533,7 @@ version_loop_for_if_conversion (struct loop *loop) struct loop *new_loop; gimple *g; gimple_stmt_iterator gsi; + unsigned int save_length; g = gimple_build_call_internal (IFN_LOOP_VECTORIZED, 2, build_int_cst (integer_type_node, loop->num), @@ -2540,8 +2541,9 @@ version_loop_for_if_conversion (struct loop *loop) gimple_call_set_lhs (g, cond); /* Save BB->aux around loop_version as that uses the same field. */ - void **saved_preds = XALLOCAVEC (void *, loop->num_nodes); - for (unsigned i = 0; i < loop->num_nodes; i++) + save_length = loop->inner ? loop->inner->num_nodes : loop->num_nodes; + void **saved_preds = XALLOCAVEC (void *, save_length); + for (unsigned i = 0; i < save_length; i++) saved_preds[i] = ifc_bbs[i]->aux; initialize_original_copy_tables (); @@ -2550,7 +2552,7 @@ version_loop_for_if_conversion (struct loop *loop) REG_BR_PROB_BASE, true); free_original_copy_tables (); - for (unsigned i = 0; i < loop->num_nodes; i++) + for (unsigned i = 0; i < save_length; i++) ifc_bbs[i]->aux = saved_preds[i]; if (new_loop == NULL) @@ -2565,6 +2567,40 @@ version_loop_for_if_conversion (struct loop *loop) return true; } +/* Return true when LOOP satisfies the follow conditions that will + allow it to be recognized by the vectorizer for outer-loop + vectorization: + - The loop has exactly one inner loop. + - The loop has a single exit. + - The loop header has a single successor, which is the inner + loop header. + - The loop exit block has a single predecessor, which is the + inner loop's exit block. */ + +static bool +versionable_outer_loop_p (struct loop *loop) +{ + if (!loop->inner + || loop->inner->next + || !single_exit (loop) + || !single_succ_p (loop->header) + || single_succ (loop->header) != loop->inner->header) + return false; + + gcc_assert (single_pred_p (loop->latch)); + basic_block outer_exit = single_pred (loop->latch); + gcc_assert (single_pred_p (loop->inner->latch)); + basic_block inner_exit = single_pred (loop->inner->latch); + + if (!single_pred_p (outer_exit) || single_pred (outer_exit) != inner_exit) + return false; + + if (dump_file) + fprintf (dump_file, "Found versionable outer loop\n"); + + return true; +} + /* Performs splitting of critical edges. Skip splitting and return false if LOOP will not be converted because: @@ -2767,8 +2803,16 @@ tree_if_conversion (struct loop *loop) || loop->dont_vectorize)) goto cleanup; - if ((any_pred_load_store || any_complicated_phi) - && !version_loop_for_if_conversion (loop)) + /* Since we have no cost model, always version loops if vectorization + is enabled. Either version this loop, or if the pattern is right + for outer-loop vectorization, version the outer loop. In the + latter case we will still if-convert the original inner loop. */ + /* FIXME: When SLP vectorization can handle if-conversion on its own, + predicate all of if-conversion on flag_tree_loop_vectorize. */ + if ((any_pred_load_store || any_complicated_phi || flag_tree_loop_vectorize) + && !version_loop_for_if_conversion + (versionable_outer_loop_p (loop_outer (loop)) + ? loop_outer (loop) : loop)) goto cleanup; /* Now all statements are if-convertible. Combine all the basic