From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64853 invoked by alias); 14 May 2018 21:18:16 -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 64821 invoked by uid 89); 14 May 2018 21:18:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=nest, HX-Received:sk:z8-v6mr, preventing X-HELO: mail-yb0-f194.google.com Received: from mail-yb0-f194.google.com (HELO mail-yb0-f194.google.com) (209.85.213.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 14 May 2018 21:18:13 +0000 Received: by mail-yb0-f194.google.com with SMTP id 140-v6so4634088ybc.9 for ; Mon, 14 May 2018 14:18:13 -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:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=mfjDts7jV0IuaP8OmfcdT5jPE/j3TK9M/J6PdaF1aCk=; b=NOM5Bu0ARk5oXG9W65aLSnsGfo625UvjFOj2LMkrIRGzlAg8w84rcxLURrxL+oNkl+ jwzrskm5ZA572EiP1x55JeskVjs3BVFt2iz/s5c3EIKfuoEjB4vQG0/o+zfHWI24EBwb 0mtwXeFjpR5UuSxBGcH3vEBxiHOQZmocfeRn1cVcZ85fHA7sp3NLFPUkcSk90dObSl7G mjKi0TkIAmAdH4LmN3tcRiPlwPSMdD3wOjlyry0GGHwbo6PtGA7a4TLr1GZo4PggUOGE IdZQpQZbRB/H74+euZxzMI9g/hToThUbP/UNm5/SDM+Vfikxil57Dr9mIFOjP6Uesltg xDUg== X-Gm-Message-State: ALKqPwe80/qtdk1kMgQrBIH9Ebima8lybYrIeQ11yUDn66AtcRXkFtx9 YZlPJO5QPjCHtDFTFcRDHh7lCQ== X-Google-Smtp-Source: AB8JxZpXBHfSNfzYL1AsKapWpQ8k+6y1/3gRcLV1nxQO/k7+WSmp/p8ednsopDmZsCJCRqcWb8uJQw== X-Received: by 2002:a25:ba48:: with SMTP id z8-v6mr7624609ybj.110.1526332691462; Mon, 14 May 2018 14:18:11 -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 k3-v6sm4121068ywd.63.2018.05.14.14.18.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 14 May 2018 14:18:10 -0700 (PDT) Subject: Re: [PATCH 1/2] Introduce prefetch-minimum stride option From: Luis Machado To: gcc-patches@gcc.gnu.org Cc: james.greenhalgh@arm.com, Richard.Earnshaw@arm.com, Kyrill Tkachov , "H.J. Lu" , Jeff Law References: <1516628770-25036-1-git-send-email-luis.machado@linaro.org> <1516628770-25036-2-git-send-email-luis.machado@linaro.org> Message-ID: <468c1099-3a87-6e95-53c4-3ba62fe3472f@linaro.org> Date: Mon, 14 May 2018 21:21: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: <1516628770-25036-2-git-send-email-luis.machado@linaro.org> Content-Type: multipart/mixed; boundary="------------340BE83F0B6DF3CDC9290CC0" X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00651.txt.bz2 This is a multi-part message in MIME format. --------------340BE83F0B6DF3CDC9290CC0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1563 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. 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? --------------340BE83F0B6DF3CDC9290CC0 Content-Type: text/x-patch; name="0001-Introduce-prefetch-minimum-stride-option.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Introduce-prefetch-minimum-stride-option.patch" Content-length: 8801 >From e0207950a6d7793cdaceaa71fc5ada05a93dc1b3 Mon Sep 17 00:00:00 2001 From: Luis Machado Date: Thu, 21 Dec 2017 15:23:59 -0200 Subject: [PATCH 1/2] Introduce prefetch-minimum stride option 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 and x86_64-linux. Ok? 2018-05-14 Luis Machado Introduce option to limit software prefetching to known constant strides above a specific threshold with the goal of preventing conflicts with a hardware prefetcher. gcc/ * config/aarch64/aarch64-protos.h (cpu_prefetch_tune) : New const int field. * config/aarch64/aarch64.c (generic_prefetch_tune): Update to include minimum_stride field. (exynosm1_prefetch_tune): Likewise. (thunderxt88_prefetch_tune): Likewise. (thunderx_prefetch_tune): Likewise. (thunderx2t99_prefetch_tune): Likewise. (qdf24xx_prefetch_tune): Likewise. Set minimum_stride to 2048. : Set to 3. (aarch64_override_options_internal): Update to set PARAM_PREFETCH_MINIMUM_STRIDE. * doc/invoke.texi (prefetch-minimum-stride): Document new option. * params.def (PARAM_PREFETCH_MINIMUM_STRIDE): New. * params.h (PARAM_PREFETCH_MINIMUM_STRIDE): Define. * tree-ssa-loop-prefetch.c (should_issue_prefetch_p): Return false if stride is constant and is below the minimum stride threshold. --- gcc/config/aarch64/aarch64-protos.h | 3 +++ gcc/config/aarch64/aarch64.c | 13 ++++++++++++- gcc/doc/invoke.texi | 15 +++++++++++++++ gcc/params.def | 9 +++++++++ gcc/params.h | 2 ++ gcc/tree-ssa-loop-prefetch.c | 17 +++++++++++++++++ 6 files changed, 58 insertions(+), 1 deletion(-) diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index cda2895..5d3b9d7 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -230,6 +230,9 @@ struct cpu_prefetch_tune const int l1_cache_size; const int l1_cache_line_size; const int l2_cache_size; + /* The minimum constant stride beyond which we should use prefetch + hints for. */ + const int minimum_stride; const int default_opt_level; }; diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index a2003fe..5215deb 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -547,6 +547,7 @@ static const cpu_prefetch_tune generic_prefetch_tune = -1, /* l1_cache_size */ -1, /* l1_cache_line_size */ -1, /* l2_cache_size */ + -1, /* minimum_stride */ -1 /* default_opt_level */ }; @@ -556,6 +557,7 @@ static const cpu_prefetch_tune exynosm1_prefetch_tune = -1, /* l1_cache_size */ 64, /* l1_cache_line_size */ -1, /* l2_cache_size */ + -1, /* minimum_stride */ -1 /* default_opt_level */ }; @@ -565,7 +567,8 @@ static const cpu_prefetch_tune qdf24xx_prefetch_tune = 32, /* l1_cache_size */ 64, /* l1_cache_line_size */ 512, /* l2_cache_size */ - -1 /* default_opt_level */ + 2048, /* minimum_stride */ + 3 /* default_opt_level */ }; static const cpu_prefetch_tune thunderxt88_prefetch_tune = @@ -574,6 +577,7 @@ static const cpu_prefetch_tune thunderxt88_prefetch_tune = 32, /* l1_cache_size */ 128, /* l1_cache_line_size */ 16*1024, /* l2_cache_size */ + -1, /* minimum_stride */ 3 /* default_opt_level */ }; @@ -583,6 +587,7 @@ static const cpu_prefetch_tune thunderx_prefetch_tune = 32, /* l1_cache_size */ 128, /* l1_cache_line_size */ -1, /* l2_cache_size */ + -1, /* minimum_stride */ -1 /* default_opt_level */ }; @@ -592,6 +597,7 @@ static const cpu_prefetch_tune thunderx2t99_prefetch_tune = 32, /* l1_cache_size */ 64, /* l1_cache_line_size */ 256, /* l2_cache_size */ + -1, /* minimum_stride */ -1 /* default_opt_level */ }; @@ -10617,6 +10623,11 @@ aarch64_override_options_internal (struct gcc_options *opts) aarch64_tune_params.prefetch->l2_cache_size, opts->x_param_values, global_options_set.x_param_values); + if (aarch64_tune_params.prefetch->minimum_stride >= 0) + maybe_set_param_value (PARAM_PREFETCH_MINIMUM_STRIDE, + aarch64_tune_params.prefetch->minimum_stride, + opts->x_param_values, + global_options_set.x_param_values); /* Use the alternative scheduling-pressure algorithm by default. */ maybe_set_param_value (PARAM_SCHED_PRESSURE_ALGORITHM, SCHED_PRESSURE_MODEL, diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 6019e1f..c92153e 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -10718,6 +10718,21 @@ The size of L1 cache, in kilobytes. @item l2-cache-size The size of L2 cache, in kilobytes. +@item prefetch-minimum-stride +Minimum constant stride, in bytes, to start using prefetch hints for. If +the stride is less than this threshold, prefetch hints will not be issued. + +This setting is useful for processors that have hardware prefetchers, in +which case there may be conflicts between the hardware prefetchers and +the software prefetchers. If the hardware prefetchers have a maximum +stride they can handle, it should be used here to improve the use of +software prefetchers. + +A value of -1, the default, means we don't have a threshold and therefore +prefetch hints can be issued for any constant stride. + +This setting is only useful for strides that are known and constant. + @item loop-interchange-max-num-stmts The maximum number of stmts in a loop to be interchanged. diff --git a/gcc/params.def b/gcc/params.def index dad47ec..2166deb 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -790,6 +790,15 @@ DEFPARAM (PARAM_L2_CACHE_SIZE, "The size of L2 cache.", 512, 0, 0) +/* The minimum constant stride beyond which we should use prefetch hints + for. */ + +DEFPARAM (PARAM_PREFETCH_MINIMUM_STRIDE, + "prefetch-minimum-stride", + "The minimum constant stride beyond which we should use prefetch " + "hints for.", + -1, 0, 0) + /* Maximum number of statements in loop nest for loop interchange. */ DEFPARAM (PARAM_LOOP_INTERCHANGE_MAX_NUM_STMTS, diff --git a/gcc/params.h b/gcc/params.h index 98249d2..96012db 100644 --- a/gcc/params.h +++ b/gcc/params.h @@ -196,6 +196,8 @@ extern void init_param_values (int *params); PARAM_VALUE (PARAM_L1_CACHE_LINE_SIZE) #define L2_CACHE_SIZE \ PARAM_VALUE (PARAM_L2_CACHE_SIZE) +#define PREFETCH_MINIMUM_STRIDE \ + PARAM_VALUE (PARAM_PREFETCH_MINIMUM_STRIDE) #define USE_CANONICAL_TYPES \ PARAM_VALUE (PARAM_USE_CANONICAL_TYPES) #define IRA_MAX_LOOPS_NUM \ diff --git a/gcc/tree-ssa-loop-prefetch.c b/gcc/tree-ssa-loop-prefetch.c index 2f10db1..e9d8e5f 100644 --- 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) + { + 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", + ref->group->uid, ref->uid, int_cst_value (ref->group->step), + PREFETCH_MINIMUM_STRIDE); + return false; + } + /* For now do not issue prefetches for only first few of the iterations. */ if (ref->prefetch_before != PREFETCH_ALL) -- 2.7.4 --------------340BE83F0B6DF3CDC9290CC0--