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

[Luis Machado <luis.machado@linaro.org>]

On 05/16/2018 06:08 AM, Kyrill Tkachov wrote:
> 
> 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?

The changes are not substantial, but midend approval i what i was aiming at.

Also the confirmation that PR85682 is no longer happening.

Luis