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

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

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.

> Thanks,
> Luis
>
> On 01/22/2018 11:46 AM, Luis Machado wrote:
>> This patch adds a new option to control the minimum stride, for a memory
>> reference, after which the loop prefetch pass may issue software prefetch
>> hints for. There are two motivations:
>>
>> * Make the pass less aggressive, only issuing prefetch hints for bigger strides
>> that are more likely to benefit from prefetching. I've noticed a case in cpu2017
>> where we were issuing thousands of hints, for example.
>>
>> * For processors that have a hardware prefetcher, like Falkor, it allows the
>> loop prefetch pass to defer prefetching of smaller (less than the threshold)
>> strides to the hardware prefetcher instead. This prevents conflicts between
>> the software prefetcher and the hardware prefetcher.
>>
>> I've noticed considerable reduction in the number of prefetch hints and
>> slightly positive performance numbers. This aligns GCC and LLVM in terms of
>> prefetch behavior for Falkor.
>>
>> The default settings should guarantee no changes for existing targets. Those
>> are free to tweak the settings as necessary.
>>
>> No regressions in the testsuite and bootstrapped ok on aarch64-linux.
>>
>> Ok?
>
>

--- 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.

Thanks,
Kyrill