On Mon, Jun 13, 2016 at 12:01 PM, Bin Cheng <bin.ch...@arm.com> wrote:
> Hi,
> GCC vectorizer generates many unnecessary runtime alias checks known at 
> compilation time.  For some data-reference pairs, alias relation can be 
> resolved at compilation time, in this case, we can simply skip the alias 
> check.  For some other data-reference pairs, alias relation can be realized 
> at compilation time, in this case, we should return false and simply skip 
> vectorizing the loop.  For the second case, the corresponding loop is 
> vectorized for nothing because the guard alias condition is bound to false 
> anyway.  Vectorizing it not only wastes compilation time, but also slows down 
> generated code because GCC fails to resolve these "false" alias check after 
> vectorization.  Even in cases it can be resolved (by VRP), GCC fails to 
> cleanup all the mess generated in loop versioning.
> This looks like a common issue in spec2k6.  For example, in 
> 434.zeusmp/ggen.f, there are three loops vectorized but never executed; in 
> 464.h264ref, there are loops in which all runtime alias checks are resolved 
> at compilation time thus loop versioning is proven unnecessary.  Statistic 
> data also shows that about >100 loops are falsely vectorized currently in my 
> build of spec2k6.
>
> This patch is based on 
> https://gcc.gnu.org/ml/gcc-patches/2016-06/msg00399.html, bootstrap and test 
> on x86_64 and AArch64 (ongoing), is it OK?

This is PR71060 and PR65206?

Rather than patching up vect_prune_runtime_alias_test_list to handle
runtime known _not_ aliasing (does that happen?  for one of the
testcases?) I'd patch vect_analyze_data_ref_dependence for that case.
Yes, we can have cases where the runtime check generated
is imprecise enough that it is proved to always alias, thus handling
these cases seems fine (in which case handling the other is
trivial).

So I'm generally fine with the patch but please check the above PRs
and eventually add testcases from them.

+/* Function vect_no_alias_p.
+
+   Given data references A and B whose alias relation is known at

A and B with equal base and offset

+   compilation time, return TRUE if they do not alias to each other;
+   return FALSE if they do.  SEGMENT_LENGTH_A and SEGMENT_LENGTH_B
+   are the memory lengths accessed by A and B respectively.  */
+
+static bool
+vect_no_alias_p (struct data_reference *a, struct data_reference *b,
+                 tree segment_length_a, tree segment_length_b)

please don't mix tree and wide_int so much.  Either use wide_int exclusively
throughout the function or use tree_int_cst_eq for equality and
tree_int_cst_le for the <= compares.

+      comp_res = compare_tree (DR_BASE_ADDRESS (dr_a), DR_BASE_ADDRESS (dr_b));
+      if (comp_res == 0)
+       comp_res = compare_tree (DR_OFFSET (dr_a), DR_OFFSET (dr_b));
+
+      /* Alias is known at compilation time.  */
+      if (comp_res == 0
+         && TREE_CODE (length_factor) == INTEGER_CST
+         && TREE_CODE (DR_STEP (dr_a)) == INTEGER_CST
+         && TREE_CODE (DR_STEP (dr_b)) == INTEGER_CST)
+       {
+         if (vect_no_alias_p (dr_a, dr_b, segment_length_a, segment_length_b))
+           continue;

I wonder if you'd rather want to check segment_length_a/b for INTEGER_CST
(not length_factor).

Thanks,
Richard.

> Thanks,
> bin
>
> 2016-06-07  Bin Cheng  <bin.ch...@arm.com>
>
>         * tree-vect-data-refs.c (vect_no_alias_p): New function.
>         (vect_prune_runtime_alias_test_list): Call vect_no_alias_p to
>         resolve alias checks which are known at compilation time.
>         Truncate vector LOOP_VINFO_MAY_ALIAS_DDRS(loop_vinfo) if all
>         alias checks are resolved at compilation time.

Reply via email to