Re: [PATCH/RFC] Make loop-header-copying more aggressive, rerun before tree-if-conversion

[Alan Lawrence <alan.lawrence@arm.com>]

Richard Biener wrote:
> Apart from Jeffs comment - the usual fix for the undesired
> vectorization is to put
> a __asm__ volatile (""); in the loop.

In vect-strided-a-u16-i4.c, narrowing the scope of the declaration seemed to 
preserve the original intent. I've been able to drop the other testsuite changes.

> +  /* If any block in the loop has an exit edge, and code after it, it is
> +     not a do-while loop.  */
> +  basic_block *body = get_loop_body (loop);
> +  for (unsigned i = 0; i < loop->num_nodes; i++)
> 
> wouldn't it be easier to verify that the predecessor of the loop latch
> contains the (only) loop exit?

It's not guaranteed that the loop latch has only one predecessor. The testsuite 
contains quite a few examples, e.g. gcc.c-torture/compile/20011114.c (at -O3). 
However, I've found a simpler (and equivalent) test, as we have the unique exit 
edge and it's source already.

> Note that single_exit () only works when the loop state has
> LOOPS_HAVE_RECORDED_EXITS

Hah, thanks - didn't realize that. So using single_exit_p did make pass_ch 
behave differently from pass_ch_vect. I've restored the original code for the 
original pass_ch...

 > I think pass_ch_vect
 > should be only executed if flag_tree_loop_vectorize is enabled.

...agreed; and handling loop->force_vectorize and loop->dont_vectorize properly 
required splitting the two phases up more anyway, so I've used clearly-different 
predicates in each.

> Coding-style wise, can you please move the "common" pass_ch_vect::execute out
> of the pass_ch_vect class?

Yes, I've done some reorg, introducing a third base class with the common 
execute bits calling a virtual method returning bool.

>    loop_optimizer_init (LOOPS_NORMAL
> -                      | LOOPS_HAVE_RECORDED_EXITS);
> +                      | LOOPS_HAVE_RECORDED_EXITS
> +                      | LOOPS_HAVE_PREHEADERS
> +                      | LOOPS_HAVE_SIMPLE_LATCHES);
> 
> already included in LOOPS_NORMAL.

So it is. Thanks!

TYVM for the review - I've posted a v2 at 
https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01355.html .

Cheers, Alan