On 05/23/2018 05:01 PM, H.J. Lu wrote:
On Tue, May 22, 2018 at 11:55 AM, Luis Machado <luis.mach...@linaro.org> wrote:
On 05/16/2018 08:18 AM, Luis Machado wrote:
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.
James confirmed PR85682 is no longer reproducible with the updated patch and
the bootstrap issue is fixed now. So i take it this should be OK to push to
mainline?
Also, i'd like to discuss the possibility of having these couple options
backported to GCC 8. As is, the changes don't alter code generation by
default, but they allow better tuning of the software prefetcher for targets
that benefit from it.
Maybe after letting the changes bake on mainline enough to be confirmed
stable?
It breaks GCC bootstrap on i686:
../../src-trunk/gcc/tree-ssa-loop-prefetch.c: In function ‘bool
should_issue_prefetch_p(mem_ref*)’:
../../src-trunk/gcc/tree-ssa-loop-prefetch.c:1015:4: error: format
‘%ld’ expects argument of type ‘long int’, but argument 5 has type
‘long long int’ [-Werror=format=]
"Step for reference %u:%u (%ld) is less than the mininum "
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"required stride of %d\n",
~~~~~~~~~~~~~~~~~~~~~~~~~
ref->group->uid, ref->uid, int_cst_value (ref->group->step),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Sorry. Does the following fix it for i686?
I had bootstrapped it for x86_64.
2018-05-22 Luis Machado <luis.mach...@linaro.org>
* tree-ssa-loop-prefetch.c (should_issue_prefetch_p): Cast value
to HOST_WIDE_INT.
Index: gcc/tree-ssa-loop-prefetch.c
===================================================================
--- gcc/tree-ssa-loop-prefetch.c (revision 260625)
+++ gcc/tree-ssa-loop-prefetch.c (working copy)
@@ -1014,7 +1014,8 @@
fprintf (dump_file,
"Step for reference %u:%u (%ld) is less than the mininum "
"required stride of %d\n",
- ref->group->uid, ref->uid, int_cst_value (ref->group->step),
+ ref->group->uid, ref->uid,
+ (HOST_WIDE_INT) int_cst_value (ref->group->step),
PREFETCH_MINIMUM_STRIDE);
return false;
}
