Re: [PATCH 1/2] Introduce prefetch-minimum stride option

[Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>]

On 15/05/18 12:12, Luis Machado wrote:
> Hi,
>
> On 05/15/2018 06:37 AM, Kyrill Tkachov wrote:
>> Hi Luis,
>>
>> On 14/05/18 22:18, Luis Machado wrote:
>>> Hi,
>>>
>>> Here's an updated version of the patch (now reverted) that addresses the previous bootstrap problem (signedness and long long/int conversion).
>>>
>>> I've checked that it bootstraps properly on both aarch64-linux and x86_64-linux and that tests look sane.
>>>
>>> James, would you please give this one a try to see if you can still reproduce PR85682? I couldn't reproduce it in multiple attempts.
>>>
>>
>> The patch doesn't hit the regressions in PR85682 from what I can see.
>> I have a comment on the patch below.
>>
>
> Great. Thanks for checking Kyrill.
>
>> --- a/gcc/tree-ssa-loop-prefetch.c
>> +++ b/gcc/tree-ssa-loop-prefetch.c
>> @@ -992,6 +992,23 @@ prune_by_reuse (struct mem_ref_group *groups)
>>   static bool
>>   should_issue_prefetch_p (struct mem_ref *ref)
>>   {
>> +  /* Some processors may have a hardware prefetcher that may conflict with
>> +     prefetch hints for a range of strides.  Make sure we don't issue
>> +     prefetches for such cases if the stride is within this particular
>> +     range.  */
>> +  if (cst_and_fits_in_hwi (ref->group->step)
>> +      && abs_hwi (int_cst_value (ref->group->step)) <
>> +      (HOST_WIDE_INT) PREFETCH_MINIMUM_STRIDE)
>> +    {
>>
>> The '<' should go on the line below together with PREFETCH_MINIMUM_STRIDE.
>
> I've fixed this locally now.

Thanks. I haven't followed the patch in detail, are you looking for midend changes approval since the last version?
Or do you need aarch64 approval?

Kyrill