From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106408 invoked by alias); 23 May 2018 22:35:45 -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 106399 invoked by uid 89); 23 May 2018 22:35:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-Received:sk:v15-v6m X-HELO: mail-ot0-f193.google.com Received: from mail-ot0-f193.google.com (HELO mail-ot0-f193.google.com) (74.125.82.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 23 May 2018 22:35:43 +0000 Received: by mail-ot0-f193.google.com with SMTP id l22-v6so27161225otj.0 for ; Wed, 23 May 2018 15:35:43 -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=TvOIWXbnOEbcRIZLV0pHw45xfl7UjWiaefYsUoTwPHw=; b=dYUE/UnCjaROSz6KRj9RUzjsBWoq7E+lM8urhl+8t6+fMlxAyjV8uOV8AsHiobnSTC oZ+3r0mKtI7RhIE+EequHGbImp94xNaHcYORixvEs7ahl2KOTdfQLS7rLOxZNeOjzM8S 2w5Jw6coZxUmDXFnuv/U5hR/HdlfQpt9qprDQfJ/QukCO7bK1oqthOKkR5TB8mffvNbs LRyPzlmmoPnJ444nTvj0UX/vFyN3K7gptxw7JzEuJG3k7P/cglxqn1tdm5HyZSo35z75 NHrg0k2TLSV4UDKC6soq6I8vJxu4zfdpi8Y+jgdWeBGo2k6oTlAFSt3rVABxt6CRVUqu zrbQ== X-Gm-Message-State: ALKqPwczyhPgbqq/D/hXU766qY6QuXnYLG/mPCdZpq32NuqkXzcnKx68 BqDGEU+lEwiF4UuOQ0X1cxLYAFfjJbCwpbpMKzA= X-Google-Smtp-Source: AB8JxZrq0+s66ktG0kQroNd9PWlgvPhk0WAbHXscPOKzxtpCoWgHoR7N0hG6mGzckRSjeN6LoZGnAFXKlQTF6J4LcJs= X-Received: by 2002:a9d:118f:: with SMTP id v15-v6mr3132498otf.125.1527114941839; Wed, 23 May 2018 15:35:41 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a4a:2e12:0:0:0:0:0 with HTTP; Wed, 23 May 2018 15:35:41 -0700 (PDT) In-Reply-To: <7ea9a127-1ae6-9fa6-6cd4-06818a3d7b8b@linaro.org> 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:41: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/msg01307.txt.bz2 On Wed, May 23, 2018 at 3:29 PM, Luis Machado wro= te: > > > 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 address= es >>>>>>>> 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 se= e. >>>>>>> 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 aimi= ng >>>> 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 =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); --=20 H.J.