From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 93023 invoked by alias); 23 May 2018 22:29:28 -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 92989 invoked by uid 89); 23 May 2018 22:29:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=AWL,BAYES_00,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-yb0-f195.google.com Received: from mail-yb0-f195.google.com (HELO mail-yb0-f195.google.com) (209.85.213.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 23 May 2018 22:29:25 +0000 Received: by mail-yb0-f195.google.com with SMTP id d123-v6so734463ybh.9 for ; Wed, 23 May 2018 15:29:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=tJMkFy1mH2HePZdRP0CjBNxWBMkYLlIt2V5PtO+y+7k=; b=A4gWWvjjJgVSvH4Q5ElBfvE3f02OLU2dTtANBMSbxM8g2N2BzWucxWzbRWwmClZqy9 rlmv6kpXXREJdPe5N0Mv1J/AbFWSXcrMN51Cpl7iZ/z8l0bczbbRIXwu4yhlzEMRFM/5 Qu5K3dvxadREGA58qa/MYs0HHARPRmumBufxJM8olFMDsdX/j9avvMVBRxrfrzd+QOb7 RICoZBF1xHYHBTnYJ6pqn5gJ+RicFBQIYJHhNTs0NgOqhr/dCO506rHzjxuW65s4rXjS +fqSLf8d//vCqG3RPapqjMg5KjdPvW2zxbiLax34E9UYeNiZOPq1vyfAqo5/kvmLj5zr G3QA== X-Gm-Message-State: ALKqPwcRL8+35IAaKI2XCQIVVUWXBTQhO8W8CBSjmqeRcjfwbRZV4PlR G6RD2eo5tuT7WNGWTpeyk5VQOw== X-Google-Smtp-Source: AB8JxZqyLCQU4tcGD9tw98G0hcI6s+1jzvfjsJublvvAOl0nSfGTseEQXPrWCQGerZpiWAWoislZJg== X-Received: by 2002:a25:ade8:: with SMTP id d40-v6mr2633443ybe.123.1527114563383; Wed, 23 May 2018 15:29:23 -0700 (PDT) Received: from [192.168.1.15] (c-69-243-238-236.hsd1.al.comcast.net. [69.243.238.236]) by smtp.gmail.com with ESMTPSA id d23-v6sm8585688ywb.69.2018.05.23.15.29.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 23 May 2018 15:29:22 -0700 (PDT) Subject: Re: [PATCH 1/2] Introduce prefetch-minimum stride option To: "H.J. Lu" Cc: Kyrill Tkachov , GCC Patches , James Greenhalgh , Richard Earnshaw , Jeff Law 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> From: Luis Machado Message-ID: <7ea9a127-1ae6-9fa6-6cd4-06818a3d7b8b@linaro.org> Date: Wed, 23 May 2018 22:34:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------A1A5F213433274C358CFCE87" X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg01305.txt.bz2 This is a multi-part message in MIME format. --------------A1A5F213433274C358CFCE87 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 3568 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 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. --------------A1A5F213433274C358CFCE87 Content-Type: text/x-patch; name="i686_bootstrap.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="i686_bootstrap.diff" Content-length: 688 2018-05-22 Luis Machado * 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; } --------------A1A5F213433274C358CFCE87--