On Tue, Jun 28, 2016 at 8:18 AM, Bin.Cheng <amker.ch...@gmail.com> wrote:
> On Tue, Jun 14, 2016 at 1:57 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> 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.
> Hi,
> The two PRs you mentioned is the case which is proved to always alias.
> Before this patch, the loop is vectorized, alias check is generated
> and then simplified into false, at last, the versioned loop gets
> deleted. With this patch, it proves always alias earlier and we won't
> do the useless versioning any more. And I checked spec, there are
> quite a lot compile time no-alias cases.
>
> Here is the updated patch wrto your comments. Is it OK?
Ok.
Thanks,
Richard.
> Thanks,
> bin
>
>>
>> +/* 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.
