From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69646 invoked by alias); 23 May 2018 22:41:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 69602 invoked by uid 89); 23 May 2018 22:41:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-oi0-f68.google.com Received: from mail-oi0-f68.google.com (HELO mail-oi0-f68.google.com) (209.85.218.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 23 May 2018 22:41:03 +0000 Received: by mail-oi0-f68.google.com with SMTP id 11-v6so21006351ois.8 for ; Wed, 23 May 2018 15:41:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=QGxBdQkuJ1K7anSnKpgdXx+M4Em199RfbHnA+H2alnk=; b=QfBYPyecWmoAgbadS+Y4jfoE/McuCNYSVvaTsJZkJS+TegxfSkzp6VVkG5ul3Od5rz oHekFGjmdQ/ssNzSYixQPBtqQtfWtnh2pAtIyOSazzGZRKaNQjjgeBQIKIlWj9r8SOaA /35iS4OWRBQht/6thkHtieL18cIhP/Hbyhjn3UNi69Yn5QYvL/oi8Iyw1Y7QvZpMHkgH zfriGXRTztZCiCcpudUd/Dkd0+m4NZwk1ESnSrxEHr8Mx86vkFH53ks9D99PtoK0Jhwu SNNjExD+9W4ScXmHCLxR3MHVTEAI4DKITmczMHVmd18zuFhmwQHsfP5NrCLQ+dHG3EXb 0nPQ== X-Gm-Message-State: ALKqPwcrwYtteTs/9JvorXRCI6421lv0j2Fp0/4pRMfR0G7hLaVLr+3j QYz4RTIKXdmTzVoFXemCaLAKvEO6LSut2DdFLZE= X-Google-Smtp-Source: AB8JxZqI+7kPr8/t9tfNttMJCT/KgHTrp+ZF3sWxn7dkFRlykgtfB6o/lGiJ1YdpVKRBybhJC/DoMMLMKR3C9O1g+oU= X-Received: by 2002:aca:c3c9:: with SMTP id t192-v6mr2467636oif.311.1527115261753; Wed, 23 May 2018 15:41:01 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a4a:2e12:0:0:0:0:0 with HTTP; Wed, 23 May 2018 15:41:01 -0700 (PDT) In-Reply-To: References: <1516628770-25036-1-git-send-email-luis.machado@linaro.org> <1516628770-25036-2-git-send-email-luis.machado@linaro.org> <468c1099-3a87-6e95-53c4-3ba62fe3472f@linaro.org> <5AFAAA57.2080501@foss.arm.com> <3b4ec84e-5902-f7de-f047-282c3b3fff08@linaro.org> <5AFBF522.5040805@foss.arm.com> <41528915-fb50-7c59-f476-f5c085621bc8@linaro.org> <12caffd9-81c4-1a29-429a-6fd712db08d8@linaro.org> <7ea9a127-1ae6-9fa6-6cd4-06818a3d7b8b@linaro.org> From: "H.J. Lu" Date: Wed, 23 May 2018 22:42:00 -0000 Message-ID: Subject: Re: [PATCH 1/2] Introduce prefetch-minimum stride option To: Luis Machado Cc: Kyrill Tkachov , GCC Patches , James Greenhalgh , Richard Earnshaw , Jeff Law Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg01308.txt.bz2 On Wed, May 23, 2018 at 3:35 PM, H.J. Lu wrote: > On Wed, May 23, 2018 at 3:29 PM, Luis Machado w= rote: >> >> >> On 05/23/2018 05:01 PM, H.J. Lu wrote: >>> >>> On Tue, May 22, 2018 at 11:55 AM, Luis Machado >>> 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 addres= ses >>>>>>>>> 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 sti= ll >>>>>>>>> reproduce PR85682? I couldn't reproduce it in multiple attempts. >>>>>>>>> >>>>>>>> >>>>>>>> The patch doesn't hit the regressions in PR85682 from what I can s= ee. >>>>>>>> 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 aim= ing >>>>> at. >>>>> >>>>> Also the confirmation that PR85682 is no longer happening. >>>> >>>> >>>> >>>> James confirmed PR85682 is no longer reproducible with the updated pat= ch >>>> and >>>> the bootstrap issue is fixed now. So i take it this should be OK to pu= sh >>>> to >>>> mainline? >>>> >>>> Also, i'd like to discuss the possibility of having these couple optio= ns >>>> 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 =E2=80=98bool >>> should_issue_prefetch_p(mem_ref*)=E2=80=99: >>> ../../src-trunk/gcc/tree-ssa-loop-prefetch.c:1015:4: error: format >>> =E2=80=98%ld=E2=80=99 expects argument of type =E2=80=98long int=E2=80= =99, but argument 5 has type >>> =E2=80=98long long int=E2=80=99 [-Werror=3Dformat=3D] >>> "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? >> > > Index: gcc/tree-ssa-loop-prefetch.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 " > ^^^ Please use > HOST_WIDE_INT_PRINT_DEC > "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); > > Something like this. --=20 H.J. --- diff --git a/gcc/tree-ssa-loop-prefetch.c b/gcc/tree-ssa-loop-prefetch.c index c3e7fd1e529..949a67f360e 100644 --- a/gcc/tree-ssa-loop-prefetch.c +++ b/gcc/tree-ssa-loop-prefetch.c @@ -1012,8 +1012,8 @@ should_issue_prefetch_p (struct mem_ref *ref) { if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, - "Step for reference %u:%u (%ld) is less than the mininum " - "required stride of %d\n", + "Step for reference %u:%u (" HOST_WIDE_INT_PRINT_C + ") is less than the mininum required stride of %d\n", ref->group->uid, ref->uid, int_cst_value (ref->group->step), PREFETCH_MINIMUM_STRIDE); return false;