[{"id":1767691,"web_url":"http://patchwork.ozlabs.org/comment/1767691/","msgid":"<59B8F43B.6090701@foss.arm.com>","list_archive_url":null,"date":"2017-09-13T09:02:51","subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","submitter":{"id":66284,"url":"http://patchwork.ozlabs.org/api/people/66284/","name":"Kyrill Tkachov","email":"kyrylo.tkachov@foss.arm.com"},"content":"Hi Charles,\n\nOn 12/09/17 09:34, charles.baylis@linaro.org wrote:\n> From: Charles Baylis <charles.baylis@linaro.org>\n>\n> This patch moves the calculation of costs for MEM into a\n> separate function, and reforms the calculation into two\n> parts. Firstly any additional cost of the addressing mode\n> is calculated, and then the cost of the memory access itself\n> is added.\n>\n> In this patch, the calculation of the cost of the addressing\n> mode is left as a placeholder, to be added in a subsequent\n> patch.\n>\n\nCan you please mention how has this series been tested?\nA bootstrap and test run on arm-none-linux-gnueabihf is required at least.\nAlso, do you have any benchmarking results for this?\nI agree that generating the addressing modes in the new tests is desirable.\nSo I'm not objecting to the goal of this patch, but a check to make sure \nthat this doesn't regress SPEC\nwould be great.  Further comments on the patch inline.\n\n> gcc/ChangeLog:\n>\n> <date>  Charles Baylis <charles.baylis@linaro.org>\n>\n>         * config/arm/arm.c (arm_mem_costs): New function.\n>         (arm_rtx_costs_internal): Use arm_mem_costs.\n>\n> gcc/testsuite/ChangeLog:\n>\n> <date>  Charles Baylis <charles.baylis@linaro.org>\n>\n>         * gcc.target/arm/addr-modes-float.c: New test.\n>         * gcc.target/arm/addr-modes-int.c: New test.\n>         * gcc.target/arm/addr-modes.h: New header.\n>\n> Change-Id: I99e93406ea39ee31f71c7bf428ad3e127b7a618e\n> ---\n>  gcc/config/arm/arm.c                            | 67 \n> ++++++++++++++++---------\n>  gcc/testsuite/gcc.target/arm/addr-modes-float.c | 42 ++++++++++++++++\n>  gcc/testsuite/gcc.target/arm/addr-modes-int.c   | 46 +++++++++++++++++\n>  gcc/testsuite/gcc.target/arm/addr-modes.h       | 53 +++++++++++++++++++\n>  4 files changed, 183 insertions(+), 25 deletions(-)\n>  create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes-float.c\n>  create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes-int.c\n>  create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes.h\n>\n> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c\n> index 32001e5..b8dbed6 100644\n> --- a/gcc/config/arm/arm.c\n> +++ b/gcc/config/arm/arm.c\n> @@ -9228,8 +9228,48 @@ arm_unspec_cost (rtx x, enum rtx_code /* \n> outer_code */, bool speed_p, int *cost)\n> } \\\n>          while (0);\n>\n> +/* Helper function for arm_rtx_costs_internal.  Calculates the cost \n> of a MEM,\n> +   considering the costs of the addressing mode and memory access\n> +   separately.  */\n> +static bool\n> +arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,\n> +              int *cost, bool speed_p)\n> +{\n> +  machine_mode mode = GET_MODE (x);\n> +  if (flag_pic\n> +      && GET_CODE (XEXP (x, 0)) == PLUS\n> +      && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n> +    /* This will be split into two instructions.  Add the cost of the\n> +       additional instruction here.  The cost of the memory access is \n> computed\n> +       below.  See arm.md:calculate_pic_address.  */\n> +    *cost = COSTS_N_INSNS (1);\n> +  else\n> +    *cost = 0;\n\nFor speed_p we want the size cost of the insn (COSTS_N_INSNS (1) for a \neach insn)\nplus the appropriate field in extra_cost. So you should unconditionally \ninitialise the cost\nto COSTS_N_INSNS (1), conditionally increment it by COSTS_N_INSNS (1) \nwith the condition above.\n\n> +\n> +  /* Calculate cost of the addressing mode.  */\n> +  if (speed_p)\n> +    {\n> +      /* TODO: Add table-driven costs for addressing modes.  (See \n> patch 2) */\n> +    }\n\nYou mean \"patch 3\". I recommend you just remove this conditional from \nthis patch and add the logic\nin patch 3 entirely.\n\n> +\n> +  /* Calculate cost of memory access.  */\n> +  if (speed_p)\n> +    {\n> +      /* data transfer is transfer size divided by bus width.  */\n> +      int bus_width_bytes = current_tune->bus_width / 4;\n\nThis should be bus_width / BITS_PER_UNIT to get the size in bytes.\nBITS_PER_UNIT is 8 though, so you'll have to double check to make sure the\ncost calculation and generated code is still appropriate.\n\n> +      *cost += CEIL (GET_MODE_SIZE (mode), bus_width_bytes);\n> +      *cost += extra_cost->ldst.load;\n> +    }\n> +  else\n> +    {\n> +      *cost += COSTS_N_INSNS (1);\n> +    }\n\nGiven my first comment above this else would be deleted.\n\nThanks,\nKyrill\n\n> +\n> +  return true;\n> +}\n> +\n>  /* RTX costs.  Make an estimate of the cost of executing the operation\n> -   X, which is contained with an operation with code OUTER_CODE.\n> +   X, which is contained within an operation with code OUTER_CODE.\n>     SPEED_P indicates whether the cost desired is the performance cost,\n>     or the size cost.  The estimate is stored in COST and the return\n>     value is TRUE if the cost calculation is final, or FALSE if the\n> @@ -9308,30 +9348,7 @@ arm_rtx_costs_internal (rtx x, enum rtx_code \n> code, enum rtx_code outer_code,\n>        return false;\n>\n>      case MEM:\n> -      /* A memory access costs 1 insn if the mode is small, or the \n> address is\n> -        a single register, otherwise it costs one insn per word.  */\n> -      if (REG_P (XEXP (x, 0)))\n> -       *cost = COSTS_N_INSNS (1);\n> -      else if (flag_pic\n> -              && GET_CODE (XEXP (x, 0)) == PLUS\n> -              && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n> -       /* This will be split into two instructions.\n> -          See arm.md:calculate_pic_address.  */\n> -       *cost = COSTS_N_INSNS (2);\n> -      else\n> -       *cost = COSTS_N_INSNS (ARM_NUM_REGS (mode));\n> -\n> -      /* For speed optimizations, add the costs of the address and\n> -        accessing memory.  */\n> -      if (speed_p)\n> -#ifdef NOT_YET\n> -       *cost += (extra_cost->ldst.load\n> -                 + arm_address_cost (XEXP (x, 0), mode,\n> -                                     ADDR_SPACE_GENERIC, speed_p));\n> -#else\n> -        *cost += extra_cost->ldst.load;\n> -#endif\n> -      return true;\n> +      return arm_mem_costs (x, extra_cost, cost, speed_p);\n>\n>      case PARALLEL:\n>      {\n> diff --git a/gcc/testsuite/gcc.target/arm/addr-modes-float.c \n> b/gcc/testsuite/gcc.target/arm/addr-modes-float.c\n> new file mode 100644\n> index 0000000..3b4235c\n> --- /dev/null\n> +++ b/gcc/testsuite/gcc.target/arm/addr-modes-float.c\n> @@ -0,0 +1,42 @@\n> +/* { dg-options \"-O2\" } */\n> +/* { dg-add-options arm_neon } */\n> +/* { dg-require-effective-target arm_neon_ok } */\n> +/* { dg-do compile } */\n> +\n> +#include <arm_neon.h>\n> +\n> +#include \"addr-modes.h\"\n> +\n> +POST_STORE(float)\n> +/* { dg-final { scan-assembler \"vstmia.32\" } } */\n> +POST_STORE(double)\n> +/* { dg-final { scan-assembler \"vstmia.64\" } } */\n> +\n> +POST_LOAD(float)\n> +/* { dg-final { scan-assembler \"vldmia.32\" } } */\n> +POST_LOAD(double)\n> +/* { dg-final { scan-assembler \"vldmia.64\" } } */\n> +\n> +POST_STORE_VEC (int8_t, int8x8_t, vst1_s8)\n> +/* { dg-final { scan-assembler \"vst1.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } \n> } */\n> +POST_STORE_VEC (int8_t, int8x16_t, vst1q_s8)\n> +/* { dg-final { scan-assembler \"vst1.8\\t\\{.*\\[-,\\]d.*\\}, \n> \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n> +\n> +POST_STORE_VEC (int8_t, int8x8x2_t, vst2_s8)\n> +/* { dg-final { scan-assembler \"vst2.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } \n> } */\n> +POST_STORE_VEC (int8_t, int8x16x2_t, vst2q_s8)\n> +/* { dg-final { scan-assembler \"vst2.8\\t\\{.*-d.*\\}, \n> \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n> +\n> +POST_STORE_VEC (int8_t, int8x8x3_t, vst3_s8)\n> +/* { dg-final { scan-assembler \"vst3.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } \n> } */\n> +POST_STORE_VEC (int8_t, int8x16x3_t, vst3q_s8)\n> +/* { dg-final { scan-assembler \"vst3.8\\t\\{d\\[02468\\], d\\[02468\\], \n> d\\[02468\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n> +/* { dg-final { scan-assembler \"vst3.8\\t\\{d\\[13579\\], d\\[13579\\], \n> d\\[13579\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" { xfail *-*-* } } } */\n> +\n> +POST_STORE_VEC (int8_t, int8x8x4_t, vst4_s8)\n> +/* { dg-final { scan-assembler \"vst4.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } \n> } */\n> +POST_STORE_VEC (int8_t, int8x16x4_t, vst4q_s8)\n> +/* { dg-final { scan-assembler \"vst4.8\\t\\{d\\[02468\\], d\\[02468\\], \n> d\\[02468\\], d\\[02468\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n> +/* { dg-final { scan-assembler \"vst4.8\\t\\{d\\[13579\\], d\\[13579\\], \n> d\\[13579\\], d\\[13579\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" { xfail *-*-* } } } */\n> +\n> +/* { dg-final { scan-assembler-not \"add\" { xfail *-*-* } } } */\n> diff --git a/gcc/testsuite/gcc.target/arm/addr-modes-int.c \n> b/gcc/testsuite/gcc.target/arm/addr-modes-int.c\n> new file mode 100644\n> index 0000000..e3e1e6a\n> --- /dev/null\n> +++ b/gcc/testsuite/gcc.target/arm/addr-modes-int.c\n> @@ -0,0 +1,46 @@\n> +/* { dg-options \"-O2 -march=armv7-a\" } */\n> +/* { dg-add-options arm_neon } */\n> +/* { dg-require-effective-target arm_neon_ok } */\n> +/* { dg-do compile } */\n> +\n> +#include \"addr-modes.h\"\n> +\n> +typedef long long ll;\n> +\n> +PRE_STORE(char)\n> +/* { dg-final { scan-assembler \"strb.*#1]!\" } } */\n> +PRE_STORE(short)\n> +/* { dg-final { scan-assembler \"strh.*#2]!\" } } */\n> +PRE_STORE(int)\n> +/* { dg-final { scan-assembler \"str.*#4]!\" } } */\n> +PRE_STORE(ll)\n> +/* { dg-final { scan-assembler \"strd.*#8]!\" } } */\n> +\n> +POST_STORE(char)\n> +/* { dg-final { scan-assembler \"strb.*], #1\" } } */\n> +POST_STORE(short)\n> +/* { dg-final { scan-assembler \"strh.*], #2\" } } */\n> +POST_STORE(int)\n> +/* { dg-final { scan-assembler \"str.*], #4\" } } */\n> +POST_STORE(ll)\n> +/* { dg-final { scan-assembler \"strd.*], #8\" } } */\n> +\n> +PRE_LOAD(char)\n> +/* { dg-final { scan-assembler \"ldrb.*#1]!\" } } */\n> +PRE_LOAD(short)\n> +/* { dg-final { scan-assembler \"ldrsh.*#2]!\" } } */\n> +PRE_LOAD(int)\n> +/* { dg-final { scan-assembler \"ldr.*#4]!\" } } */\n> +PRE_LOAD(ll)\n> +/* { dg-final { scan-assembler \"ldrd.*#8]!\" } } */\n> +\n> +POST_LOAD(char)\n> +/* { dg-final { scan-assembler \"ldrb.*], #1\" } } */\n> +POST_LOAD(short)\n> +/* { dg-final { scan-assembler \"ldrsh.*], #2\" } } */\n> +POST_LOAD(int)\n> +/* { dg-final { scan-assembler \"ldr.*], #4\" } } */\n> +POST_LOAD(ll)\n> +/* { dg-final { scan-assembler \"ldrd.*], #8\" } } */\n> +\n> +/* { dg-final { scan-assembler-not \"\\tadd\" } } */\n> diff --git a/gcc/testsuite/gcc.target/arm/addr-modes.h \n> b/gcc/testsuite/gcc.target/arm/addr-modes.h\n> new file mode 100644\n> index 0000000..eac4678\n> --- /dev/null\n> +++ b/gcc/testsuite/gcc.target/arm/addr-modes.h\n> @@ -0,0 +1,53 @@\n> +\n> +#define PRE_STORE(T)                   \\\n> +  T *                                  \\\n> +  T ## _pre_store (T *p, T v)          \\\n> +  {                                    \\\n> +    *++p = v;                          \\\n> +    return p;                          \\\n> +  }                                    \\\n> +\n> +#define POST_STORE(T)                  \\\n> +  T *                                  \\\n> +  T ## _post_store (T *p, T v)         \\\n> +  {                                    \\\n> +    *p++ = v;                          \\\n> +    return p;                          \\\n> +  }\n> +\n> +#define POST_STORE_VEC(T, VT, OP)      \\\n> +  T *                                  \\\n> +  VT ## _post_store (T * p, VT v)      \\\n> +  {                                    \\\n> +    OP (p, v);                         \\\n> +    p += sizeof (VT) / sizeof (T);     \\\n> +    return p;                          \\\n> +  }\n> +\n> +#define PRE_LOAD(T)                    \\\n> +  void                                 \\\n> +  T ## _pre_load (T *p)                        \\\n> +  {                                    \\\n> +    extern void f ## T (T*,T);         \\\n> +    T x = *++p;                                \\\n> +    f ## T (p, x);                     \\\n> +  }\n> +\n> +#define POST_LOAD(T)                   \\\n> +  void                                 \\\n> +  T ## _post_load (T *p)               \\\n> +  {                                    \\\n> +    extern void f ## T (T*,T);         \\\n> +    T x = *p++;                                \\\n> +    f ## T (p, x);                     \\\n> +  }\n> +\n> +#define POST_LOAD_VEC(T, VT, OP)       \\\n> +  void                                 \\\n> +  VT ## _post_load (T * p)             \\\n> +  {                                    \\\n> +    extern void f ## T (T*,T);         \\\n> +    VT x = OP (p, v);                  \\\n> +    p += sizeof (VT) / sizeof (T);     \\\n> +    f ## T (p, x);                     \\\n> +  }\n> -- \n> 2.7.4\n>","headers":{"Return-Path":"<gcc-patches-return-462005-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-462005-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"mAV8jJwz\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsbLD6zYHz9sMN\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 13 Sep 2017 19:03:32 +1000 (AEST)","(qmail 18256 invoked by alias); 13 Sep 2017 09:02:59 -0000","(qmail 18192 invoked by uid 89); 13 Sep 2017 09:02:58 -0000","from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com)\n\t(217.140.101.70) by sourceware.org\n\t(qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tWed, 13 Sep 2017 09:02:55 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\tby\n\tusa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id\n\t713EA1529; Wed, 13 Sep 2017 02:02:54 -0700 (PDT)","from [10.2.207.77] (e100706-lin.cambridge.arm.com\n\t[10.2.207.77])\tby usa-sjc-imap-foss1.foss.arm.com (Postfix)\n\twith ESMTPSA id 571D63F483; Wed, 13 Sep 2017 02:02:53 -0700 (PDT)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:date:from:mime-version:to:cc:subject:references\n\t:in-reply-to:content-type:content-transfer-encoding; q=dns; s=\n\tdefault; b=JfkagFqI30CMHe8A9Sw2bKtwFLSIxc9W1XmjveRs/YSdyOSuwQaA3\n\t85p8nGdlZTt3OjN9HDVdrYyv9f7ccaYdvbvxXhs6KHbrNClmN6owcclVnAA8ym8x\n\tiP/HEFqLbyRw+oaSyqyX24StxKU8GVWudA67wN3T4/lDD6ztQUIt7g=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:date:from:mime-version:to:cc:subject:references\n\t:in-reply-to:content-type:content-transfer-encoding; s=default;\n\tbh=E89oF+VJ6GwJwkGFJ1P9pK7V9MI=; b=mAV8jJwz8AOSncglJiV5Hipbykvb\n\t/MtdXudktCiNc8tvfiGCFgYfZ869spuTDv8acTe3yd3WpmS3NeSutkzm0MTpwvfl\n\ta+JtXfwzaTmeJwbq+Ahr3Rnq+GsRFgsH9JXmPMQ1Zmo3DlnTe0kgN/sxfy9q9fay\n\tfgIjVvlIMbcVnJo=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0,\n\tGIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3,\n\tKAM_LAZY_DOMAIN_SECURITY,\n\tRP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=","X-HELO":"foss.arm.com","Message-ID":"<59B8F43B.6090701@foss.arm.com>","Date":"Wed, 13 Sep 2017 10:02:51 +0100","From":"Kyrill  Tkachov <kyrylo.tkachov@foss.arm.com>","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:31.0) Gecko/20100101 Thunderbird/31.2.0","MIME-Version":"1.0","To":"\"charles.baylis@linaro.org\" <charles.baylis@linaro.org>,\n\tRichard Earnshaw <Richard.Earnshaw@arm.com>,\n\tRamana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,\n\t\"pinskia@gmail.com\" <pinskia@gmail.com>","CC":"\"gcc-patches@gcc.gnu.org\" <gcc-patches@gcc.gnu.org>","Subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","References":"<1505205277-26276-1-git-send-email-charles.baylis@linaro.org>\n\t<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>","In-Reply-To":"<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Transfer-Encoding":"7bit"}},{"id":1769261,"web_url":"http://patchwork.ozlabs.org/comment/1769261/","msgid":"<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>","list_archive_url":null,"date":"2017-09-15T15:38:34","subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","submitter":{"id":35578,"url":"http://patchwork.ozlabs.org/api/people/35578/","name":"Charles Baylis","email":"charles.baylis@linaro.org"},"content":"On 13 September 2017 at 10:02, Kyrill  Tkachov\n<kyrylo.tkachov@foss.arm.com> wrote:\n> Hi Charles,\n>\n> On 12/09/17 09:34, charles.baylis@linaro.org wrote:\n>>\n>> From: Charles Baylis <charles.baylis@linaro.org>\n>>\n>> This patch moves the calculation of costs for MEM into a\n>> separate function, and reforms the calculation into two\n>> parts. Firstly any additional cost of the addressing mode\n>> is calculated, and then the cost of the memory access itself\n>> is added.\n>>\n>> In this patch, the calculation of the cost of the addressing\n>> mode is left as a placeholder, to be added in a subsequent\n>> patch.\n>>\n>\n> Can you please mention how has this series been tested?\n> A bootstrap and test run on arm-none-linux-gnueabihf is required at least.\n\nIt has been tested with make check on arm-unknown-linux-gnueabihf with\nno regressions. I've successfully bootstrapped the next spin.\n\n> Also, do you have any benchmarking results for this?\n> I agree that generating the addressing modes in the new tests is desirable.\n> So I'm not objecting to the goal of this patch, but a check to make sure\n> that this doesn't regress SPEC\n> would be great.  Further comments on the patch inline.\n\nSPEC2006 scores are unaffected by this patch on Cortex-A57.\n\n>> +/* Helper function for arm_rtx_costs_internal.  Calculates the cost of a\n>> MEM,\n>> +   considering the costs of the addressing mode and memory access\n>> +   separately.  */\n>> +static bool\n>> +arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,\n>> +              int *cost, bool speed_p)\n>> +{\n>> +  machine_mode mode = GET_MODE (x);\n>> +  if (flag_pic\n>> +      && GET_CODE (XEXP (x, 0)) == PLUS\n>> +      && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n>> +    /* This will be split into two instructions.  Add the cost of the\n>> +       additional instruction here.  The cost of the memory access is\n>> computed\n>> +       below.  See arm.md:calculate_pic_address.  */\n>> +    *cost = COSTS_N_INSNS (1);\n>> +  else\n>> +    *cost = 0;\n>\n>\n> For speed_p we want the size cost of the insn (COSTS_N_INSNS (1) for a each\n> insn)\n> plus the appropriate field in extra_cost. So you should unconditionally\n> initialise the cost\n> to COSTS_N_INSNS (1), conditionally increment it by COSTS_N_INSNS (1) with\n> the condition above.\n\nOK. I also have to subtract that COSTS_N_INSNS (1) in the if (speed_p)\npart because the cost of a single bus transfer is included in that\ninitial cost.\n\n>> +\n>> +  /* Calculate cost of the addressing mode.  */\n>> +  if (speed_p)\n>> +    {\n>> +      /* TODO: Add table-driven costs for addressing modes.  (See patch\n>> 2) */\n>> +    }\n>\n>\n> You mean \"patch 3\". I recommend you just remove this conditional from this\n> patch and add the logic\n> in patch 3 entirely.\n\nOK.\n\n>> +\n>> +  /* Calculate cost of memory access.  */\n>> +  if (speed_p)\n>> +    {\n>> +      /* data transfer is transfer size divided by bus width.  */\n>> +      int bus_width_bytes = current_tune->bus_width / 4;\n>\n>\n> This should be bus_width / BITS_PER_UNIT to get the size in bytes.\n> BITS_PER_UNIT is 8 though, so you'll have to double check to make sure the\n> cost calculation and generated code is still appropriate.\n\nOops, I changed the units around and messed this up. I'll fix this.\n\n>> +      *cost += CEIL (GET_MODE_SIZE (mode), bus_width_bytes);\n>> +      *cost += extra_cost->ldst.load;\n>> +    }\n>> +  else\n>> +    {\n>> +      *cost += COSTS_N_INSNS (1);\n>> +    }\n>\n> Given my first comment above this else would be deleted.\n\nOK\nFrom f81e1d3212475a3dc7aaeb8cb3171c6defd40687 Mon Sep 17 00:00:00 2001\nFrom: Charles Baylis <charles.baylis@linaro.org>\nDate: Wed, 8 Feb 2017 16:52:10 +0000\nSubject: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.\n\nThis patch moves the calculation of costs for MEM into a\nseparate function, and reforms the calculation into two\nparts. Firstly any additional cost of the addressing mode\nis calculated, and then the cost of the memory access itself\nis added.\n\nIn this patch, the calculation of the cost of the addressing\nmode is left as a placeholder, to be added in a subsequent\npatch.\n\ngcc/ChangeLog:\n\n<date>  Charles Baylis  <charles.baylis@linaro.org>\n\n\t* config/arm/arm.c (arm_mem_costs): New function.\n\t(arm_rtx_costs_internal): Use arm_mem_costs.\n\ngcc/testsuite/ChangeLog:\n\n<date>  Charles Baylis  <charles.baylis@linaro.org>\n\n\t* gcc.target/arm/addr-modes-float.c: New test.\n\t* gcc.target/arm/addr-modes-int.c: New test.\n\t* gcc.target/arm/addr-modes.h: New header.\n\nChange-Id: I99e93406ea39ee31f71c7bf428ad3e127b7a618e\n---\n gcc/config/arm/arm.c                            | 60 ++++++++++++++-----------\n gcc/testsuite/gcc.target/arm/addr-modes-float.c | 42 +++++++++++++++++\n gcc/testsuite/gcc.target/arm/addr-modes-int.c   | 46 +++++++++++++++++++\n gcc/testsuite/gcc.target/arm/addr-modes.h       | 53 ++++++++++++++++++++++\n 4 files changed, 176 insertions(+), 25 deletions(-)\n create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes-float.c\n create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes-int.c\n create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes.h\n\ndiff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c\nindex 32001e5..64230b8 100644\n--- a/gcc/config/arm/arm.c\n+++ b/gcc/config/arm/arm.c\n@@ -9228,8 +9228,41 @@ arm_unspec_cost (rtx x, enum rtx_code /* outer_code */, bool speed_p, int *cost)\n \t  }\t\t\t\t\t\t\t\t\\\n \twhile (0);\n \n+/* Helper function for arm_rtx_costs_internal.  Calculates the cost of a MEM,\n+   considering the costs of the addressing mode and memory access\n+   separately.  */\n+static bool\n+arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,\n+\t       int *cost, bool speed_p)\n+{\n+  machine_mode mode = GET_MODE (x);\n+\n+  *cost = COSTS_N_INSNS (1);\n+\n+  if (flag_pic\n+      && GET_CODE (XEXP (x, 0)) == PLUS\n+      && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n+    /* This will be split into two instructions.  Add the cost of the\n+       additional instruction here.  The cost of the memory access is computed\n+       below.  See arm.md:calculate_pic_address.  */\n+    *cost += COSTS_N_INSNS (1);\n+\n+  /* Calculate cost of memory access.  */\n+  if (speed_p)\n+    {\n+      /* data transfer is transfer size divided by bus width.  */\n+      int bus_width_bytes = current_tune->bus_width / BITS_PER_UNIT;\n+      int num_transfers = CEIL (GET_MODE_SIZE (mode), bus_width_bytes);\n+      /* first transfer included in initial cost */\n+      *cost += COSTS_N_INSNS (num_transfers - 1);\n+      *cost += extra_cost->ldst.load;\n+    }\n+\n+  return true;\n+}\n+\n /* RTX costs.  Make an estimate of the cost of executing the operation\n-   X, which is contained with an operation with code OUTER_CODE.\n+   X, which is contained within an operation with code OUTER_CODE.\n    SPEED_P indicates whether the cost desired is the performance cost,\n    or the size cost.  The estimate is stored in COST and the return\n    value is TRUE if the cost calculation is final, or FALSE if the\n@@ -9308,30 +9341,7 @@ arm_rtx_costs_internal (rtx x, enum rtx_code code, enum rtx_code outer_code,\n       return false;\n \n     case MEM:\n-      /* A memory access costs 1 insn if the mode is small, or the address is\n-\t a single register, otherwise it costs one insn per word.  */\n-      if (REG_P (XEXP (x, 0)))\n-\t*cost = COSTS_N_INSNS (1);\n-      else if (flag_pic\n-\t       && GET_CODE (XEXP (x, 0)) == PLUS\n-\t       && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n-\t/* This will be split into two instructions.\n-\t   See arm.md:calculate_pic_address.  */\n-\t*cost = COSTS_N_INSNS (2);\n-      else\n-\t*cost = COSTS_N_INSNS (ARM_NUM_REGS (mode));\n-\n-      /* For speed optimizations, add the costs of the address and\n-\t accessing memory.  */\n-      if (speed_p)\n-#ifdef NOT_YET\n-\t*cost += (extra_cost->ldst.load\n-\t\t  + arm_address_cost (XEXP (x, 0), mode,\n-\t\t\t\t      ADDR_SPACE_GENERIC, speed_p));\n-#else\n-        *cost += extra_cost->ldst.load;\n-#endif\n-      return true;\n+      return arm_mem_costs (x, extra_cost, cost, speed_p);\n \n     case PARALLEL:\n     {\ndiff --git a/gcc/testsuite/gcc.target/arm/addr-modes-float.c b/gcc/testsuite/gcc.target/arm/addr-modes-float.c\nnew file mode 100644\nindex 0000000..3b4235c\n--- /dev/null\n+++ b/gcc/testsuite/gcc.target/arm/addr-modes-float.c\n@@ -0,0 +1,42 @@\n+/* { dg-options \"-O2\" } */\n+/* { dg-add-options arm_neon } */\n+/* { dg-require-effective-target arm_neon_ok } */\n+/* { dg-do compile } */\n+\n+#include <arm_neon.h>\n+\n+#include \"addr-modes.h\"\n+\n+POST_STORE(float)\n+/* { dg-final { scan-assembler \"vstmia.32\" } } */\n+POST_STORE(double)\n+/* { dg-final { scan-assembler \"vstmia.64\" } } */\n+\n+POST_LOAD(float)\n+/* { dg-final { scan-assembler \"vldmia.32\" } } */\n+POST_LOAD(double)\n+/* { dg-final { scan-assembler \"vldmia.64\" } } */\n+\n+POST_STORE_VEC (int8_t, int8x8_t, vst1_s8)\n+/* { dg-final { scan-assembler \"vst1.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16_t, vst1q_s8)\n+/* { dg-final { scan-assembler \"vst1.8\\t\\{.*\\[-,\\]d.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+\n+POST_STORE_VEC (int8_t, int8x8x2_t, vst2_s8)\n+/* { dg-final { scan-assembler \"vst2.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16x2_t, vst2q_s8)\n+/* { dg-final { scan-assembler \"vst2.8\\t\\{.*-d.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+\n+POST_STORE_VEC (int8_t, int8x8x3_t, vst3_s8)\n+/* { dg-final { scan-assembler \"vst3.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16x3_t, vst3q_s8)\n+/* { dg-final { scan-assembler \"vst3.8\\t\\{d\\[02468\\], d\\[02468\\], d\\[02468\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+/* { dg-final { scan-assembler \"vst3.8\\t\\{d\\[13579\\], d\\[13579\\], d\\[13579\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" { xfail *-*-* } } } */\n+\n+POST_STORE_VEC (int8_t, int8x8x4_t, vst4_s8)\n+/* { dg-final { scan-assembler \"vst4.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16x4_t, vst4q_s8)\n+/* { dg-final { scan-assembler \"vst4.8\\t\\{d\\[02468\\], d\\[02468\\], d\\[02468\\], d\\[02468\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+/* { dg-final { scan-assembler \"vst4.8\\t\\{d\\[13579\\], d\\[13579\\], d\\[13579\\], d\\[13579\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" { xfail *-*-* } } } */\n+\n+/* { dg-final { scan-assembler-not \"add\" { xfail *-*-* } } } */\ndiff --git a/gcc/testsuite/gcc.target/arm/addr-modes-int.c b/gcc/testsuite/gcc.target/arm/addr-modes-int.c\nnew file mode 100644\nindex 0000000..e3e1e6a\n--- /dev/null\n+++ b/gcc/testsuite/gcc.target/arm/addr-modes-int.c\n@@ -0,0 +1,46 @@\n+/* { dg-options \"-O2 -march=armv7-a\" } */\n+/* { dg-add-options arm_neon } */\n+/* { dg-require-effective-target arm_neon_ok } */\n+/* { dg-do compile } */\n+\n+#include \"addr-modes.h\"\n+\n+typedef long long ll;\n+\n+PRE_STORE(char)\n+/* { dg-final { scan-assembler \"strb.*#1]!\" } } */\n+PRE_STORE(short)\n+/* { dg-final { scan-assembler \"strh.*#2]!\" } } */\n+PRE_STORE(int)\n+/* { dg-final { scan-assembler \"str.*#4]!\" } } */\n+PRE_STORE(ll)\n+/* { dg-final { scan-assembler \"strd.*#8]!\" } } */\n+\n+POST_STORE(char)\n+/* { dg-final { scan-assembler \"strb.*], #1\" } } */\n+POST_STORE(short)\n+/* { dg-final { scan-assembler \"strh.*], #2\" } } */\n+POST_STORE(int)\n+/* { dg-final { scan-assembler \"str.*], #4\" } } */\n+POST_STORE(ll)\n+/* { dg-final { scan-assembler \"strd.*], #8\" } } */\n+\n+PRE_LOAD(char)\n+/* { dg-final { scan-assembler \"ldrb.*#1]!\" } } */\n+PRE_LOAD(short)\n+/* { dg-final { scan-assembler \"ldrsh.*#2]!\" } } */\n+PRE_LOAD(int)\n+/* { dg-final { scan-assembler \"ldr.*#4]!\" } } */\n+PRE_LOAD(ll)\n+/* { dg-final { scan-assembler \"ldrd.*#8]!\" } } */\n+\n+POST_LOAD(char)\n+/* { dg-final { scan-assembler \"ldrb.*], #1\" } } */\n+POST_LOAD(short)\n+/* { dg-final { scan-assembler \"ldrsh.*], #2\" } } */\n+POST_LOAD(int)\n+/* { dg-final { scan-assembler \"ldr.*], #4\" } } */\n+POST_LOAD(ll)\n+/* { dg-final { scan-assembler \"ldrd.*], #8\" } } */\n+\n+/* { dg-final { scan-assembler-not \"\\tadd\" } } */\ndiff --git a/gcc/testsuite/gcc.target/arm/addr-modes.h b/gcc/testsuite/gcc.target/arm/addr-modes.h\nnew file mode 100644\nindex 0000000..eac4678\n--- /dev/null\n+++ b/gcc/testsuite/gcc.target/arm/addr-modes.h\n@@ -0,0 +1,53 @@\n+\n+#define PRE_STORE(T)\t\t\t\\\n+  T *\t\t\t\t\t\\\n+  T ## _pre_store (T *p, T v)\t\t\\\n+  {\t\t\t\t\t\\\n+    *++p = v;\t\t\t\t\\\n+    return p;\t\t\t\t\\\n+  }\t\t\t\t\t\\\n+\n+#define POST_STORE(T)\t\t\t\\\n+  T *\t\t\t\t\t\\\n+  T ## _post_store (T *p, T v)\t\t\\\n+  {\t\t\t\t\t\\\n+    *p++ = v;\t\t\t\t\\\n+    return p;\t\t\t\t\\\n+  }\n+\n+#define POST_STORE_VEC(T, VT, OP)\t\\\n+  T *\t\t\t\t\t\\\n+  VT ## _post_store (T * p, VT v)\t\\\n+  {\t\t\t\t\t\\\n+    OP (p, v);\t\t\t\t\\\n+    p += sizeof (VT) / sizeof (T);\t\\\n+    return p;\t\t\t\t\\\n+  }\n+\n+#define PRE_LOAD(T)\t\t\t\\\n+  void\t\t\t\t\t\\\n+  T ## _pre_load (T *p)\t\t\t\\\n+  {\t\t\t\t\t\\\n+    extern void f ## T (T*,T);\t\t\\\n+    T x = *++p;\t\t\t\t\\\n+    f ## T (p, x);\t\t\t\\\n+  }\n+\n+#define POST_LOAD(T)\t\t\t\\\n+  void\t\t\t\t\t\\\n+  T ## _post_load (T *p)\t\t\\\n+  {\t\t\t\t\t\\\n+    extern void f ## T (T*,T);\t\t\\\n+    T x = *p++;\t\t\t\t\\\n+    f ## T (p, x);\t\t\t\\\n+  }\n+\n+#define POST_LOAD_VEC(T, VT, OP)\t\\\n+  void\t\t\t\t\t\\\n+  VT ## _post_load (T * p)\t\t\\\n+  {\t\t\t\t\t\\\n+    extern void f ## T (T*,T);\t\t\\\n+    VT x = OP (p, v);\t\t\t\\\n+    p += sizeof (VT) / sizeof (T);\t\\\n+    f ## T (p, x);\t\t\t\\\n+  }","headers":{"Return-Path":"<gcc-patches-return-462262-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-462262-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"VkV95DhH\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xv0294RbRz9sPr\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSat, 16 Sep 2017 01:39:29 +1000 (AEST)","(qmail 10098 invoked by alias); 15 Sep 2017 15:38:40 -0000","(qmail 10004 invoked by uid 89); 15 Sep 2017 15:38:40 -0000","from mail-it0-f53.google.com (HELO mail-it0-f53.google.com)\n\t(209.85.214.53) by sourceware.org\n\t(qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tFri, 15 Sep 2017 15:38:37 +0000","by mail-it0-f53.google.com with SMTP id 85so3946897ith.2 for\n\t<gcc-patches@gcc.gnu.org>; Fri, 15 Sep 2017 08:38:37 -0700 (PDT)","by 10.2.96.106 with HTTP; Fri, 15 Sep 2017 08:38:34 -0700 (PDT)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; q=dns; s=default; b=slf+1JiE3jCyUk1\n\tdKlL1UUwvaAiBV4OiwCrSnRi2a2MF8lPFkBaJkh710OumfPoZ0l9ZOse17eJIltL\n\tDil0KEkEmflgJEuY5SA8DMWFA/ImGPKvZjfettJZXVAuUrKeXk7+yQ+Kmnlef8fx\n\tEmSRbWFDn1suLUy7wVmAr4jpdNiE=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; s=default; bh=Ev427ijJmP0tFz7NVCZaD\n\tNLmhrA=; b=VkV95DhHh2+iQfJj+1g1jOiEJ3mzf8BT81ORNlNTwbwLauwDUbRXN\n\t8YbuS3lTkZp+8i5LKL8yBxbVpRTm+dKAjvUeFNuQzCuqOflPs/caxHQrdSr5QnTa\n\txxqJtoZnOaMELtjTbM0FT6T3kNm39rTZTNGytFJ+5v+ZZyC3nsz79I=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-25.0 required=5.0 tests=AWL, BAYES_00,\n\tGIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3,\n\tRCVD_IN_DNSWL_LOW,\n\tSPF_PASS autolearn=ham version=3.3.2 spammy=scores, d*","X-HELO":"mail-it0-f53.google.com","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net;\n\ts=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=/Y46T8Ni2rnkEgSkoiGFfCv7TZ+8ha3cxBHIrFHaLYc=;\n\tb=eynUpWPgK2MB+avLG75/qmi93obAQ35p7+eSgLjems437Vi04q8on7N6oEO770G+1t\n\tjlIgwDacuD8rdudoHtRmEJPRh8qXeqeb0A1YfhNHPuqArsPJ8vq3jkviL/0X5WnI+pYK\n\taFJCOL0UphH0wOWfsYlbo7Rfkh/8bpxtKZuh4XdFqUAAZD/ulwSXM1jhsiEvbGb5f/5B\n\t6vPTbsRx9tcO8Si37FZ5lXk2QA2b6jMozysk3C/hetvZz/XiqROpZueU8HWzTFArjq0G\n\t2T5ZON2uBxdndiYd62Yg66Qb3OwNha/EdzDiWe4PBs7jR34AXhFIdUDajKl1uwrh4+5u\n\td8nw==","X-Gm-Message-State":"AHPjjUiLOAbtFP4my6dBKVZzCRICXm+GudEAYx5wFdckvOaVHyOZuJmc\t9L8xvNSjGfk2A2SFb1TDGXsi+X7pXx8YHqxtP9pXEg==","X-Google-Smtp-Source":"AOwi7QDTgcP8qnzE4QC6LMbEq2YWfTHXkbFb1CJkid2hs2MftVizkdEdb8SF+BmyXMhjx6gzxzPHUdZHq0oO7GAlvi4=","X-Received":"by 10.36.70.131 with SMTP id j125mr5276746itb.141.1505489915611;\n\tFri, 15 Sep 2017 08:38:35 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<59B8F43B.6090701@foss.arm.com>","References":"<1505205277-26276-1-git-send-email-charles.baylis@linaro.org>\n\t<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>\n\t<59B8F43B.6090701@foss.arm.com>","From":"Charles Baylis <charles.baylis@linaro.org>","Date":"Fri, 15 Sep 2017 16:38:34 +0100","Message-ID":"<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>","Subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","To":"Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>","Cc":"Richard Earnshaw <Richard.Earnshaw@arm.com>,\n\tRamana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,\n\t\"pinskia@gmail.com\" <pinskia@gmail.com>,\n\t\"gcc-patches@gcc.gnu.org\" <gcc-patches@gcc.gnu.org>","Content-Type":"multipart/mixed; boundary=\"001a114494d0a2f42905593c2fb1\"","X-IsSubscribed":"yes"}},{"id":1769308,"web_url":"http://patchwork.ozlabs.org/comment/1769308/","msgid":"<59BC0769.6080407@foss.arm.com>","list_archive_url":null,"date":"2017-09-15T17:01:29","subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","submitter":{"id":66284,"url":"http://patchwork.ozlabs.org/api/people/66284/","name":"Kyrill Tkachov","email":"kyrylo.tkachov@foss.arm.com"},"content":"On 15/09/17 16:38, Charles Baylis wrote:\n> On 13 September 2017 at 10:02, Kyrill  Tkachov\n> <kyrylo.tkachov@foss.arm.com> wrote:\n>> Hi Charles,\n>>\n>> On 12/09/17 09:34, charles.baylis@linaro.org wrote:\n>>> From: Charles Baylis <charles.baylis@linaro.org>\n>>>\n>>> This patch moves the calculation of costs for MEM into a\n>>> separate function, and reforms the calculation into two\n>>> parts. Firstly any additional cost of the addressing mode\n>>> is calculated, and then the cost of the memory access itself\n>>> is added.\n>>>\n>>> In this patch, the calculation of the cost of the addressing\n>>> mode is left as a placeholder, to be added in a subsequent\n>>> patch.\n>>>\n>> Can you please mention how has this series been tested?\n>> A bootstrap and test run on arm-none-linux-gnueabihf is required at least.\n> It has been tested with make check on arm-unknown-linux-gnueabihf with\n> no regressions. I've successfully bootstrapped the next spin.\n\nThanks.\n\n>> Also, do you have any benchmarking results for this?\n>> I agree that generating the addressing modes in the new tests is desirable.\n>> So I'm not objecting to the goal of this patch, but a check to make sure\n>> that this doesn't regress SPEC\n>> would be great.  Further comments on the patch inline.\n> SPEC2006 scores are unaffected by this patch on Cortex-A57.\n\nGood, thanks for checking :)\n\n>>> +/* Helper function for arm_rtx_costs_internal.  Calculates the cost of a\n>>> MEM,\n>>> +   considering the costs of the addressing mode and memory access\n>>> +   separately.  */\n>>> +static bool\n>>> +arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,\n>>> +              int *cost, bool speed_p)\n>>> +{\n>>> +  machine_mode mode = GET_MODE (x);\n>>> +  if (flag_pic\n>>> +      && GET_CODE (XEXP (x, 0)) == PLUS\n>>> +      && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n>>> +    /* This will be split into two instructions.  Add the cost of the\n>>> +       additional instruction here.  The cost of the memory access is\n>>> computed\n>>> +       below.  See arm.md:calculate_pic_address.  */\n>>> +    *cost = COSTS_N_INSNS (1);\n>>> +  else\n>>> +    *cost = 0;\n>>\n>> For speed_p we want the size cost of the insn (COSTS_N_INSNS (1) for a each\n>> insn)\n>> plus the appropriate field in extra_cost. So you should unconditionally\n>> initialise the cost\n>> to COSTS_N_INSNS (1), conditionally increment it by COSTS_N_INSNS (1) with\n>> the condition above.\n> OK. I also have to subtract that COSTS_N_INSNS (1) in the if (speed_p)\n> part because the cost of a single bus transfer is included in that\n> initial cost.\n>\n>>> +\n>>> +  /* Calculate cost of the addressing mode.  */\n>>> +  if (speed_p)\n>>> +    {\n>>> +      /* TODO: Add table-driven costs for addressing modes.  (See patch\n>>> 2) */\n>>> +    }\n>>\n>> You mean \"patch 3\". I recommend you just remove this conditional from this\n>> patch and add the logic\n>> in patch 3 entirely.\n> OK.\n>\n>>> +\n>>> +  /* Calculate cost of memory access.  */\n>>> +  if (speed_p)\n>>> +    {\n>>> +      /* data transfer is transfer size divided by bus width.  */\n>>> +      int bus_width_bytes = current_tune->bus_width / 4;\n>>\n>> This should be bus_width / BITS_PER_UNIT to get the size in bytes.\n>> BITS_PER_UNIT is 8 though, so you'll have to double check to make sure the\n>> cost calculation and generated code is still appropriate.\n> Oops, I changed the units around and messed this up. I'll fix this.\n>\n>>> +      *cost += CEIL (GET_MODE_SIZE (mode), bus_width_bytes);\n>>> +      *cost += extra_cost->ldst.load;\n>>> +    }\n>>> +  else\n>>> +    {\n>>> +      *cost += COSTS_N_INSNS (1);\n>>> +    }\n>> Given my first comment above this else would be deleted.\n> OK\n\nI have a concern about using the bus_width parameter which\nI explain in the thread for patch 1 (I don't think we need it, we should \nuse the fields in extra_cost->ldst\nmore carefully).\n\nKyrill","headers":{"Return-Path":"<gcc-patches-return-462288-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-462288-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"t7emYJ9G\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xv1s313mwz9s7m\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSat, 16 Sep 2017 03:01:42 +1000 (AEST)","(qmail 58664 invoked by alias); 15 Sep 2017 17:01:35 -0000","(qmail 58644 invoked by uid 89); 15 Sep 2017 17:01:34 -0000","from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com)\n\t(217.140.101.70) by sourceware.org\n\t(qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tFri, 15 Sep 2017 17:01:33 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\tby\n\tusa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id\n\t623BD1529; Fri, 15 Sep 2017 10:01:32 -0700 (PDT)","from [10.2.207.77] (e100706-lin.cambridge.arm.com\n\t[10.2.207.77])\tby usa-sjc-imap-foss1.foss.arm.com (Postfix)\n\twith ESMTPSA id DF12E3F3E1; Fri, 15 Sep 2017 10:01:30 -0700 (PDT)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:date:from:mime-version:to:cc:subject:references\n\t:in-reply-to:content-type:content-transfer-encoding; q=dns; s=\n\tdefault; b=U+VYC4zQSTj1IErj96rU5Vo7FcIGrCWmRIUSsYgmyquFER7YzuRWv\n\tTPkiPJfCGWxipVS6sBDlVOzmSAZaPGWWYhvRCDbYNuK2U5ifdGjb0ZVXLcnaags6\n\tVSy9L4GW/2vLAoDBqObMKVAsZQ7WmweSE7wAsxLbHPjW4zY5L40sQI=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:date:from:mime-version:to:cc:subject:references\n\t:in-reply-to:content-type:content-transfer-encoding; s=default;\n\tbh=6FGLc2sFqOY2XVHmR4AnhV/9VPU=; b=t7emYJ9GYEhLFYJjUPogoG5wAdKe\n\tJ+sMmzR41SRPSCUfJOs+4138gj6YU81i2rfe+Fcvun1v0T2xeVPBKq3SKcAbQ6Kg\n\tmFgYD2HYYpAWY4E/JI4drd0o+UoxWxYCUNhwnmtCbizCaVqr0wKy/8SrMyUmx5Kp\n\tI0mAJcJxl1UjgoQ=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-0.9 required=5.0 tests=BAYES_00,\n\tKAM_LAZY_DOMAIN_SECURITY,\n\tRP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=","X-HELO":"foss.arm.com","Message-ID":"<59BC0769.6080407@foss.arm.com>","Date":"Fri, 15 Sep 2017 18:01:29 +0100","From":"Kyrill  Tkachov <kyrylo.tkachov@foss.arm.com>","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:31.0) Gecko/20100101 Thunderbird/31.2.0","MIME-Version":"1.0","To":"Charles Baylis <charles.baylis@linaro.org>","CC":"Richard Earnshaw <Richard.Earnshaw@arm.com>,\n\tRamana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,\n\t\"pinskia@gmail.com\" <pinskia@gmail.com>,\n\t\"gcc-patches@gcc.gnu.org\" <gcc-patches@gcc.gnu.org>","Subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","References":"<1505205277-26276-1-git-send-email-charles.baylis@linaro.org>\n\t<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>\n\t<59B8F43B.6090701@foss.arm.com>\n\t<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>","In-Reply-To":"<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Transfer-Encoding":"7bit"}},{"id":1807705,"web_url":"http://patchwork.ozlabs.org/comment/1807705/","msgid":"<CADnVucDOCv9j2_oSnde7-9fsqHw-at4WtSdNgdqmL8fxSyF-pg@mail.gmail.com>","list_archive_url":null,"date":"2017-11-20T21:09:46","subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","submitter":{"id":35578,"url":"http://patchwork.ozlabs.org/api/people/35578/","name":"Charles Baylis","email":"charles.baylis@linaro.org"},"content":"On 15 September 2017 at 18:01, Kyrill  Tkachov\n<kyrylo.tkachov@foss.arm.com> wrote:\n>\n> On 15/09/17 16:38, Charles Baylis wrote:\n>>\n>> On 13 September 2017 at 10:02, Kyrill  Tkachov\n>> <kyrylo.tkachov@foss.arm.com> wrote:\n>>>\n>>> Hi Charles,\n>>>\n>>> On 12/09/17 09:34, charles.baylis@linaro.org wrote:\n>>>>\n>>>> From: Charles Baylis <charles.baylis@linaro.org>\n>>>>\n>>>> This patch moves the calculation of costs for MEM into a\n>>>> separate function, and reforms the calculation into two\n>>>> parts. Firstly any additional cost of the addressing mode\n>>>> is calculated, and then the cost of the memory access itself\n>>>> is added.\n>>>>\n>>>> In this patch, the calculation of the cost of the addressing\n>>>> mode is left as a placeholder, to be added in a subsequent\n>>>> patch.\n>>>>\n>>> Can you please mention how has this series been tested?\n>>> A bootstrap and test run on arm-none-linux-gnueabihf is required at\n>>> least.\n>>\n>> It has been tested with make check on arm-unknown-linux-gnueabihf with\n>> no regressions. I've successfully bootstrapped the next spin.\n>\n>\n> Thanks.\n>\n>>> Also, do you have any benchmarking results for this?\n>>> I agree that generating the addressing modes in the new tests is\n>>> desirable.\n>>> So I'm not objecting to the goal of this patch, but a check to make sure\n>>> that this doesn't regress SPEC\n>>> would be great.  Further comments on the patch inline.\n>>\n>> SPEC2006 scores are unaffected by this patch on Cortex-A57.\n>\n>\n> Good, thanks for checking :)\n>\n>\n>>>> +/* Helper function for arm_rtx_costs_internal.  Calculates the cost of\n>>>> a\n>>>> MEM,\n>>>> +   considering the costs of the addressing mode and memory access\n>>>> +   separately.  */\n>>>> +static bool\n>>>> +arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,\n>>>> +              int *cost, bool speed_p)\n>>>> +{\n>>>> +  machine_mode mode = GET_MODE (x);\n>>>> +  if (flag_pic\n>>>> +      && GET_CODE (XEXP (x, 0)) == PLUS\n>>>> +      && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n>>>> +    /* This will be split into two instructions.  Add the cost of the\n>>>> +       additional instruction here.  The cost of the memory access is\n>>>> computed\n>>>> +       below.  See arm.md:calculate_pic_address.  */\n>>>> +    *cost = COSTS_N_INSNS (1);\n>>>> +  else\n>>>> +    *cost = 0;\n>>>\n>>>\n>>> For speed_p we want the size cost of the insn (COSTS_N_INSNS (1) for a\n>>> each\n>>> insn)\n>>> plus the appropriate field in extra_cost. So you should unconditionally\n>>> initialise the cost\n>>> to COSTS_N_INSNS (1), conditionally increment it by COSTS_N_INSNS (1)\n>>> with\n>>> the condition above.\n>>\n>> OK. I also have to subtract that COSTS_N_INSNS (1) in the if (speed_p)\n>> part because the cost of a single bus transfer is included in that\n>> initial cost.\n>>\n>>>> +\n>>>> +  /* Calculate cost of the addressing mode.  */\n>>>> +  if (speed_p)\n>>>> +    {\n>>>> +      /* TODO: Add table-driven costs for addressing modes.  (See patch\n>>>> 2) */\n>>>> +    }\n>>>\n>>>\n>>> You mean \"patch 3\". I recommend you just remove this conditional from\n>>> this\n>>> patch and add the logic\n>>> in patch 3 entirely.\n>>\n>> OK.\n>>\n>>>> +\n>>>> +  /* Calculate cost of memory access.  */\n>>>> +  if (speed_p)\n>>>> +    {\n>>>> +      /* data transfer is transfer size divided by bus width.  */\n>>>> +      int bus_width_bytes = current_tune->bus_width / 4;\n>>>\n>>>\n>>> This should be bus_width / BITS_PER_UNIT to get the size in bytes.\n>>> BITS_PER_UNIT is 8 though, so you'll have to double check to make sure\n>>> the\n>>> cost calculation and generated code is still appropriate.\n>>\n>> Oops, I changed the units around and messed this up. I'll fix this.\n>>\n>>>> +      *cost += CEIL (GET_MODE_SIZE (mode), bus_width_bytes);\n>>>> +      *cost += extra_cost->ldst.load;\n>>>> +    }\n>>>> +  else\n>>>> +    {\n>>>> +      *cost += COSTS_N_INSNS (1);\n>>>> +    }\n>>>\n>>> Given my first comment above this else would be deleted.\n>>\n>> OK\n>\n>\n> I have a concern about using the bus_width parameter which\n> I explain in the thread for patch 1 (I don't think we need it, we should use\n> the fields in extra_cost->ldst\n> more carefully).\n\nI have modified this patch accordingly. Patch 1 is no longer needed.\n\nPasses \"make check\" (with patch 3) on arm-linux-gnueabihf with no\nregressions. Bootstrap is in progress.\n\nCan I still get this in during stage 3?\n\ngcc/ChangeLog:\n\n<date>  Charles Baylis  <charles.baylis@linaro.org>\n\n        * config/arm/arm.c (arm_mem_costs): New function.\n        (arm_rtx_costs_internal): Use arm_mem_costs.\n\ngcc/testsuite/ChangeLog:\n\n<date>  Charles Baylis  <charles.baylis@linaro.org>\n\n        * gcc.target/arm/addr-modes-float.c: New test.\n        * gcc.target/arm/addr-modes-int.c: New test.\n        * gcc.target/arm/addr-modes.h: New header.\nFrom 26d9c0839ef7318074d3fd38dca3989bd3e51d54 Mon Sep 17 00:00:00 2001\nFrom: Charles Baylis <charles.baylis@linaro.org>\nDate: Wed, 8 Feb 2017 16:52:10 +0000\nSubject: [PATCH 1/3] [ARM] Refactor costs calculation for MEM.\n\nThis patch moves the calculation of costs for MEM into a\nseparate function, and reforms the calculation into two\nparts. Firstly any additional cost of the addressing mode\nis calculated, and then the cost of the memory access itself\nis added.\n\nIn this patch, the calculation of the cost of the addressing\nmode is omitted, to be added in a subsequent patch.\n\ngcc/ChangeLog:\n\n<date>  Charles Baylis  <charles.baylis@linaro.org>\n\n\t* config/arm/arm.c (arm_mem_costs): New function.\n\t(arm_rtx_costs_internal): Use arm_mem_costs.\n\ngcc/testsuite/ChangeLog:\n\n<date>  Charles Baylis  <charles.baylis@linaro.org>\n\n\t* gcc.target/arm/addr-modes-float.c: New test.\n\t* gcc.target/arm/addr-modes-int.c: New test.\n\t* gcc.target/arm/addr-modes.h: New header.\n\nChange-Id: I99e93406ea39ee31f71c7bf428ad3e127b7a618e\n---\n gcc/config/arm/arm.c                            | 71 ++++++++++++++++---------\n gcc/testsuite/gcc.target/arm/addr-modes-float.c | 42 +++++++++++++++\n gcc/testsuite/gcc.target/arm/addr-modes-int.c   | 46 ++++++++++++++++\n gcc/testsuite/gcc.target/arm/addr-modes.h       | 53 ++++++++++++++++++\n 4 files changed, 187 insertions(+), 25 deletions(-)\n create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes-float.c\n create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes-int.c\n create mode 100644 gcc/testsuite/gcc.target/arm/addr-modes.h\n\ndiff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c\nindex 1c2f8fa..ce59d80 100644\n--- a/gcc/config/arm/arm.c\n+++ b/gcc/config/arm/arm.c\n@@ -9242,8 +9242,52 @@ arm_unspec_cost (rtx x, enum rtx_code /* outer_code */, bool speed_p, int *cost)\n \t  }\t\t\t\t\t\t\t\t\\\n \twhile (0)\n \n+/* Helper function for arm_rtx_costs_internal.  Calculates the cost of a MEM,\n+   considering the costs of the addressing mode and memory access\n+   separately.  */\n+static bool\n+arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,\n+\t       int *cost, bool speed_p)\n+{\n+  machine_mode mode = GET_MODE (x);\n+\n+  *cost = COSTS_N_INSNS (1);\n+\n+  if (flag_pic\n+      && GET_CODE (XEXP (x, 0)) == PLUS\n+      && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n+    /* This will be split into two instructions.  Add the cost of the\n+       additional instruction here.  The cost of the memory access is computed\n+       below.  See arm.md:calculate_pic_address.  */\n+    *cost += COSTS_N_INSNS (1);\n+\n+  /* Calculate cost of memory access.  */\n+  if (speed_p)\n+    {\n+      if (FLOAT_MODE_P (mode))\n+\t{\n+\t  if (GET_MODE_SIZE (mode) == 8)\n+\t    *cost += extra_cost->ldst.loadd;\n+\t  else\n+\t    *cost += extra_cost->ldst.loadf;\n+\t}\n+      else if (VECTOR_MODE_P (mode))\n+\t*cost += extra_cost->ldst.loadv;\n+      else\n+\t{\n+\t  /* Integer modes */\n+\t  if (GET_MODE_SIZE (mode) == 8)\n+\t    *cost += extra_cost->ldst.ldrd;\n+\t  else\n+\t    *cost += extra_cost->ldst.load;\n+\t}\n+    }\n+\n+  return true;\n+}\n+\n /* RTX costs.  Make an estimate of the cost of executing the operation\n-   X, which is contained with an operation with code OUTER_CODE.\n+   X, which is contained within an operation with code OUTER_CODE.\n    SPEED_P indicates whether the cost desired is the performance cost,\n    or the size cost.  The estimate is stored in COST and the return\n    value is TRUE if the cost calculation is final, or FALSE if the\n@@ -9322,30 +9366,7 @@ arm_rtx_costs_internal (rtx x, enum rtx_code code, enum rtx_code outer_code,\n       return false;\n \n     case MEM:\n-      /* A memory access costs 1 insn if the mode is small, or the address is\n-\t a single register, otherwise it costs one insn per word.  */\n-      if (REG_P (XEXP (x, 0)))\n-\t*cost = COSTS_N_INSNS (1);\n-      else if (flag_pic\n-\t       && GET_CODE (XEXP (x, 0)) == PLUS\n-\t       && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n-\t/* This will be split into two instructions.\n-\t   See arm.md:calculate_pic_address.  */\n-\t*cost = COSTS_N_INSNS (2);\n-      else\n-\t*cost = COSTS_N_INSNS (ARM_NUM_REGS (mode));\n-\n-      /* For speed optimizations, add the costs of the address and\n-\t accessing memory.  */\n-      if (speed_p)\n-#ifdef NOT_YET\n-\t*cost += (extra_cost->ldst.load\n-\t\t  + arm_address_cost (XEXP (x, 0), mode,\n-\t\t\t\t      ADDR_SPACE_GENERIC, speed_p));\n-#else\n-        *cost += extra_cost->ldst.load;\n-#endif\n-      return true;\n+      return arm_mem_costs (x, extra_cost, cost, speed_p);\n \n     case PARALLEL:\n     {\ndiff --git a/gcc/testsuite/gcc.target/arm/addr-modes-float.c b/gcc/testsuite/gcc.target/arm/addr-modes-float.c\nnew file mode 100644\nindex 0000000..3b4235c\n--- /dev/null\n+++ b/gcc/testsuite/gcc.target/arm/addr-modes-float.c\n@@ -0,0 +1,42 @@\n+/* { dg-options \"-O2\" } */\n+/* { dg-add-options arm_neon } */\n+/* { dg-require-effective-target arm_neon_ok } */\n+/* { dg-do compile } */\n+\n+#include <arm_neon.h>\n+\n+#include \"addr-modes.h\"\n+\n+POST_STORE(float)\n+/* { dg-final { scan-assembler \"vstmia.32\" } } */\n+POST_STORE(double)\n+/* { dg-final { scan-assembler \"vstmia.64\" } } */\n+\n+POST_LOAD(float)\n+/* { dg-final { scan-assembler \"vldmia.32\" } } */\n+POST_LOAD(double)\n+/* { dg-final { scan-assembler \"vldmia.64\" } } */\n+\n+POST_STORE_VEC (int8_t, int8x8_t, vst1_s8)\n+/* { dg-final { scan-assembler \"vst1.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16_t, vst1q_s8)\n+/* { dg-final { scan-assembler \"vst1.8\\t\\{.*\\[-,\\]d.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+\n+POST_STORE_VEC (int8_t, int8x8x2_t, vst2_s8)\n+/* { dg-final { scan-assembler \"vst2.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16x2_t, vst2q_s8)\n+/* { dg-final { scan-assembler \"vst2.8\\t\\{.*-d.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+\n+POST_STORE_VEC (int8_t, int8x8x3_t, vst3_s8)\n+/* { dg-final { scan-assembler \"vst3.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16x3_t, vst3q_s8)\n+/* { dg-final { scan-assembler \"vst3.8\\t\\{d\\[02468\\], d\\[02468\\], d\\[02468\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+/* { dg-final { scan-assembler \"vst3.8\\t\\{d\\[13579\\], d\\[13579\\], d\\[13579\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" { xfail *-*-* } } } */\n+\n+POST_STORE_VEC (int8_t, int8x8x4_t, vst4_s8)\n+/* { dg-final { scan-assembler \"vst4.8\\t\\{.*\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+POST_STORE_VEC (int8_t, int8x16x4_t, vst4q_s8)\n+/* { dg-final { scan-assembler \"vst4.8\\t\\{d\\[02468\\], d\\[02468\\], d\\[02468\\], d\\[02468\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" } } */\n+/* { dg-final { scan-assembler \"vst4.8\\t\\{d\\[13579\\], d\\[13579\\], d\\[13579\\], d\\[13579\\]\\}, \\\\\\[r\\[0-9\\]+\\\\\\]!\" { xfail *-*-* } } } */\n+\n+/* { dg-final { scan-assembler-not \"add\" { xfail *-*-* } } } */\ndiff --git a/gcc/testsuite/gcc.target/arm/addr-modes-int.c b/gcc/testsuite/gcc.target/arm/addr-modes-int.c\nnew file mode 100644\nindex 0000000..e3e1e6a\n--- /dev/null\n+++ b/gcc/testsuite/gcc.target/arm/addr-modes-int.c\n@@ -0,0 +1,46 @@\n+/* { dg-options \"-O2 -march=armv7-a\" } */\n+/* { dg-add-options arm_neon } */\n+/* { dg-require-effective-target arm_neon_ok } */\n+/* { dg-do compile } */\n+\n+#include \"addr-modes.h\"\n+\n+typedef long long ll;\n+\n+PRE_STORE(char)\n+/* { dg-final { scan-assembler \"strb.*#1]!\" } } */\n+PRE_STORE(short)\n+/* { dg-final { scan-assembler \"strh.*#2]!\" } } */\n+PRE_STORE(int)\n+/* { dg-final { scan-assembler \"str.*#4]!\" } } */\n+PRE_STORE(ll)\n+/* { dg-final { scan-assembler \"strd.*#8]!\" } } */\n+\n+POST_STORE(char)\n+/* { dg-final { scan-assembler \"strb.*], #1\" } } */\n+POST_STORE(short)\n+/* { dg-final { scan-assembler \"strh.*], #2\" } } */\n+POST_STORE(int)\n+/* { dg-final { scan-assembler \"str.*], #4\" } } */\n+POST_STORE(ll)\n+/* { dg-final { scan-assembler \"strd.*], #8\" } } */\n+\n+PRE_LOAD(char)\n+/* { dg-final { scan-assembler \"ldrb.*#1]!\" } } */\n+PRE_LOAD(short)\n+/* { dg-final { scan-assembler \"ldrsh.*#2]!\" } } */\n+PRE_LOAD(int)\n+/* { dg-final { scan-assembler \"ldr.*#4]!\" } } */\n+PRE_LOAD(ll)\n+/* { dg-final { scan-assembler \"ldrd.*#8]!\" } } */\n+\n+POST_LOAD(char)\n+/* { dg-final { scan-assembler \"ldrb.*], #1\" } } */\n+POST_LOAD(short)\n+/* { dg-final { scan-assembler \"ldrsh.*], #2\" } } */\n+POST_LOAD(int)\n+/* { dg-final { scan-assembler \"ldr.*], #4\" } } */\n+POST_LOAD(ll)\n+/* { dg-final { scan-assembler \"ldrd.*], #8\" } } */\n+\n+/* { dg-final { scan-assembler-not \"\\tadd\" } } */\ndiff --git a/gcc/testsuite/gcc.target/arm/addr-modes.h b/gcc/testsuite/gcc.target/arm/addr-modes.h\nnew file mode 100644\nindex 0000000..eac4678\n--- /dev/null\n+++ b/gcc/testsuite/gcc.target/arm/addr-modes.h\n@@ -0,0 +1,53 @@\n+\n+#define PRE_STORE(T)\t\t\t\\\n+  T *\t\t\t\t\t\\\n+  T ## _pre_store (T *p, T v)\t\t\\\n+  {\t\t\t\t\t\\\n+    *++p = v;\t\t\t\t\\\n+    return p;\t\t\t\t\\\n+  }\t\t\t\t\t\\\n+\n+#define POST_STORE(T)\t\t\t\\\n+  T *\t\t\t\t\t\\\n+  T ## _post_store (T *p, T v)\t\t\\\n+  {\t\t\t\t\t\\\n+    *p++ = v;\t\t\t\t\\\n+    return p;\t\t\t\t\\\n+  }\n+\n+#define POST_STORE_VEC(T, VT, OP)\t\\\n+  T *\t\t\t\t\t\\\n+  VT ## _post_store (T * p, VT v)\t\\\n+  {\t\t\t\t\t\\\n+    OP (p, v);\t\t\t\t\\\n+    p += sizeof (VT) / sizeof (T);\t\\\n+    return p;\t\t\t\t\\\n+  }\n+\n+#define PRE_LOAD(T)\t\t\t\\\n+  void\t\t\t\t\t\\\n+  T ## _pre_load (T *p)\t\t\t\\\n+  {\t\t\t\t\t\\\n+    extern void f ## T (T*,T);\t\t\\\n+    T x = *++p;\t\t\t\t\\\n+    f ## T (p, x);\t\t\t\\\n+  }\n+\n+#define POST_LOAD(T)\t\t\t\\\n+  void\t\t\t\t\t\\\n+  T ## _post_load (T *p)\t\t\\\n+  {\t\t\t\t\t\\\n+    extern void f ## T (T*,T);\t\t\\\n+    T x = *p++;\t\t\t\t\\\n+    f ## T (p, x);\t\t\t\\\n+  }\n+\n+#define POST_LOAD_VEC(T, VT, OP)\t\\\n+  void\t\t\t\t\t\\\n+  VT ## _post_load (T * p)\t\t\\\n+  {\t\t\t\t\t\\\n+    extern void f ## T (T*,T);\t\t\\\n+    VT x = OP (p, v);\t\t\t\\\n+    p += sizeof (VT) / sizeof (T);\t\\\n+    f ## T (p, x);\t\t\t\\\n+  }","headers":{"Return-Path":"<gcc-patches-return-467480-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-467480-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"JPxKAchM\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yghF551R6z9t20\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 21 Nov 2017 08:10:00 +1100 (AEDT)","(qmail 74027 invoked by alias); 20 Nov 2017 21:09:53 -0000","(qmail 74009 invoked by uid 89); 20 Nov 2017 21:09:51 -0000","from mail-yw0-f178.google.com (HELO mail-yw0-f178.google.com)\n\t(209.85.161.178) by sourceware.org\n\t(qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tMon, 20 Nov 2017 21:09:48 +0000","by mail-yw0-f178.google.com with SMTP id d2so4830217ywb.11 for\n\t<gcc-patches@gcc.gnu.org>; Mon, 20 Nov 2017 13:09:48 -0800 (PST)","by 10.129.182.1 with HTTP; Mon, 20 Nov 2017 13:09:46 -0800 (PST)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; q=dns; s=default; b=GqUlHNWZnIA9H8Z\n\taFpzshP29qvutrO3UIL66GD0jI6o2szzDWaB9pUzKbiC1E6aU7nukwCVSc9uOt1V\n\twJTwE6t47vl0BDgTb8yyoLbtZ5uafdRqZajuhUz+i780To2m1qrV9qB+yoK7pOWD\n\t3Qen9iJiNFW34t5jHyx9WWjzTpH0=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; s=default; bh=LWQWJWCL0Fsfs86zpomVt\n\tgiZy4I=; b=JPxKAchMTR3mK3jlnbQKxw3Km+fXMK+Vvgrx/VoMpkZBSNDJTAEZ/\n\tT8RYkCQYPfM2Ny94vrj7tj9mOIRRqkRpNY1iAh+HIqdbvILl4Ks+yBGe0SHacN/+\n\tDRNKKBdAG4bmpC/QP5c76pzIccFPnLSg7QqonzWN2lp8im4d0SklDM=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-25.8 required=5.0 tests=AWL, BAYES_00,\n\tGIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3,\n\tKB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_NONE,\n\tSPF_PASS autolearn=ham version=3.3.2 spammy=scores, UD:arm.md,\n\td*, arm.md","X-HELO":"mail-yw0-f178.google.com","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net;\n\ts=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=7bn0NPUrXJFRWN3J84GqWtsfAkkaaaWM3ccaM5oYvR0=;\n\tb=VEqyQ/Evn2gFNBIEpzUfvTofAxUya9zEDeM/kSK4Po4VsggJEO0uYNzwRYVO56orFu\n\toQUO62k/6Z5OeQvDNJhwD7Z9IIr0sOHn08A6TOMmvdUvCnuNYRmIMlSP2D672AyKNB7K\n\tzLkZT8ijForlZTGBuuRVW9rEZANHMOHYAWxVd7gJ46fx3RUfAbtDN5/0rhd+4l+tNh5d\n\tEMhwjW54sEBAVYj85GicrnWwr2e9RbIM+XjsjP6ilE9LXYVz9SkWV2B+G8G+m7EzYbNK\n\tAE7EAcsyeqW1MChEm9Do2JScyEoza1/U4xEeE3uGsxAEpSiewZ9LXxrGPRqHhVEQ1jzs\n\tFaJA==","X-Gm-Message-State":"AJaThX5Hdrf9LkMupdAJ1bx8jqVJVa/vu6mzrmPiV2mVD7JgxYs33ifS\tMHLUhM1Vdxb0SwOC7qmJhItz12HcnQPkf0cg8YyImA==","X-Google-Smtp-Source":"AGs4zMZFavO2ampFgfAOw3jiiakbFSpxXR2AKmMvFaSVwtl0nt+qs/HqdyaBsfaJI1+gpc9yMbiLEviqsMob7l2BbaI=","X-Received":"by 10.129.175.17 with SMTP id n17mr9533413ywh.199.1511212186917;\n\tMon, 20 Nov 2017 13:09:46 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<59BC0769.6080407@foss.arm.com>","References":"<1505205277-26276-1-git-send-email-charles.baylis@linaro.org>\n\t<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>\n\t<59B8F43B.6090701@foss.arm.com>\n\t<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>\n\t<59BC0769.6080407@foss.arm.com>","From":"Charles Baylis <charles.baylis@linaro.org>","Date":"Mon, 20 Nov 2017 21:09:46 +0000","Message-ID":"<CADnVucDOCv9j2_oSnde7-9fsqHw-at4WtSdNgdqmL8fxSyF-pg@mail.gmail.com>","Subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","To":"Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>","Cc":"Richard Earnshaw <Richard.Earnshaw@arm.com>,\n\tRamana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,\n\t\"pinskia@gmail.com\" <pinskia@gmail.com>,\n\t\"gcc-patches@gcc.gnu.org\" <gcc-patches@gcc.gnu.org>","Content-Type":"multipart/mixed; boundary=\"f403045eb77a951e26055e70813b\"","X-IsSubscribed":"yes"}},{"id":1807932,"web_url":"http://patchwork.ozlabs.org/comment/1807932/","msgid":"<CADnVucBjm6rUbdhFcLwmWVtw9Rm9w6Kev1SsTRnnWGbXU_Tv5w@mail.gmail.com>","list_archive_url":null,"date":"2017-11-21T09:51:16","subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","submitter":{"id":35578,"url":"http://patchwork.ozlabs.org/api/people/35578/","name":"Charles Baylis","email":"charles.baylis@linaro.org"},"content":"On 20 November 2017 at 21:09, Charles Baylis <charles.baylis@linaro.org> wrote:\n\n> I have modified this patch accordingly. Patch 1 is no longer needed.\n>\n> Passes \"make check\" (with patch 3) on arm-linux-gnueabihf with no\n> regressions. Bootstrap is in progress.\n\nBootstrap built successfully using qemu host.\n\n> Can I still get this in during stage 3?\n>\n> gcc/ChangeLog:\n>\n> <date>  Charles Baylis  <charles.baylis@linaro.org>\n>\n>         * config/arm/arm.c (arm_mem_costs): New function.\n>         (arm_rtx_costs_internal): Use arm_mem_costs.\n>\n> gcc/testsuite/ChangeLog:\n>\n> <date>  Charles Baylis  <charles.baylis@linaro.org>\n>\n>         * gcc.target/arm/addr-modes-float.c: New test.\n>         * gcc.target/arm/addr-modes-int.c: New test.\n>         * gcc.target/arm/addr-modes.h: New header.","headers":{"Return-Path":"<gcc-patches-return-467527-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-467527-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"D3JC/Arh\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yh17m6HWCz9s5L\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 21 Nov 2017 20:51:30 +1100 (AEDT)","(qmail 42647 invoked by alias); 21 Nov 2017 09:51:20 -0000","(qmail 42636 invoked by uid 89); 21 Nov 2017 09:51:20 -0000","from mail-yw0-f195.google.com (HELO mail-yw0-f195.google.com)\n\t(209.85.161.195) by sourceware.org\n\t(qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tTue, 21 Nov 2017 09:51:18 +0000","by mail-yw0-f195.google.com with SMTP id q37so5448817ywa.12 for\n\t<gcc-patches@gcc.gnu.org>; Tue, 21 Nov 2017 01:51:18 -0800 (PST)","by 10.129.182.1 with HTTP; Tue, 21 Nov 2017 01:51:16 -0800 (PST)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; q=dns; s=default; b=kdke7UfPQuKlwBI\n\taEE8B96JmvNFCnsajL6ePzUF46n8myATqytYIg4UKPcYXYeE0/mK93Ob8E4KpwvQ\n\tljtRJZJLNN0VEMVSmcBWMjml7mhfEJvGeS/Iy+xQLdrrFQTMcxlRZv72PInjB530\n\thwSNJ+aVwGRNMhpYxaa2Q68n0rR4=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; s=default; bh=e1l+CpTQVY1t7OHRVxHCi\n\taqGHkg=; b=D3JC/ArhOxhYqyDUSqcTfBAEavua7CHmYyqtEekp5fzdsPA0fLuaT\n\tObmSYbWNZ1Sw+zkBXPWwRYJ3xOg2nv/fM8e5bDX9SO4NZfw/1r/LRYry1U07ZJeo\n\tbNmK364/8aYf8dypbADqi3RLwopHgYfO/mXzMOiym8uSDpXIfPy4FU=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-3.3 required=5.0 tests=AWL, BAYES_00,\n\tKB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_NONE,\n\tSPF_PASS autolearn=no version=3.3.2 spammy=","X-HELO":"mail-yw0-f195.google.com","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net;\n\ts=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=B8BgS2s3uXWVRmFezGPGYFUMVibJ175DFDf0i+EmHCs=;\n\tb=fAnMm5qh7yYQ46CUcWhFLaYUXF/ToOf9mKwmJuQNNlFL+AJ8bsJMRc9g6raOGAdXaa\n\tjhvP5BWM1RUOSkeXtLC7tgGa2Ld2RGEnjHhX5Vdp7umIgQJEGVoS0yd/EgZvAn60Qkb/\n\tmAsethCBbnAFGY45tdGwi7AmFFgHaGySb9HEXpZyvYq9jnDSpqrkgEdjZLIscS3nSTXk\n\tnH3yYuRtjKx7akrON++bPumGh8hwHnCG16ICgbIs92fxMhFX74Hyg7yPtzdz14yeruEK\n\tNLGUKIynCiCH+kOHLA8ri0B97oSdS+5MMylTDb1Y+z7qtTkpW8E2OgbG/Vq9yzC+bEgq\n\tCLBQ==","X-Gm-Message-State":"AJaThX4IwgMP9xPRWUKdP8ZWkpEuk84S5M8VP8+HOPd5RB/+zjWnJBds\tyiR3T3MXmQkPjXte+s4Z/R+KBGGa0NgyaTW3MAQeYQ==","X-Google-Smtp-Source":"AGs4zMYZTIGheRjwseOVQJzc86xlqW7uCHfj1nIw5KPpxzYbFk+iuUglk+nIp/q2pA+nrQTBbk3oUQ5a6jiYT0ryU+c=","X-Received":"by 10.13.227.5 with SMTP id m5mr10679221ywe.387.1511257876945;\n\tTue, 21 Nov 2017 01:51:16 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<CADnVucDOCv9j2_oSnde7-9fsqHw-at4WtSdNgdqmL8fxSyF-pg@mail.gmail.com>","References":"<1505205277-26276-1-git-send-email-charles.baylis@linaro.org>\n\t<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>\n\t<59B8F43B.6090701@foss.arm.com>\n\t<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>\n\t<59BC0769.6080407@foss.arm.com>\n\t<CADnVucDOCv9j2_oSnde7-9fsqHw-at4WtSdNgdqmL8fxSyF-pg@mail.gmail.com>","From":"Charles Baylis <charles.baylis@linaro.org>","Date":"Tue, 21 Nov 2017 09:51:16 +0000","Message-ID":"<CADnVucBjm6rUbdhFcLwmWVtw9Rm9w6Kev1SsTRnnWGbXU_Tv5w@mail.gmail.com>","Subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","To":"Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>","Cc":"Richard Earnshaw <Richard.Earnshaw@arm.com>,\n\tRamana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,\n\t\"pinskia@gmail.com\" <pinskia@gmail.com>,\n\t\"gcc-patches@gcc.gnu.org\" <gcc-patches@gcc.gnu.org>","Content-Type":"text/plain; charset=\"UTF-8\"","X-IsSubscribed":"yes"}},{"id":1809129,"web_url":"http://patchwork.ozlabs.org/comment/1809129/","msgid":"<5A169C6B.8050509@foss.arm.com>","list_archive_url":null,"date":"2017-11-23T10:01:15","subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","submitter":{"id":66284,"url":"http://patchwork.ozlabs.org/api/people/66284/","name":"Kyrill Tkachov","email":"kyrylo.tkachov@foss.arm.com"},"content":"Hi Charles,\n\nOn 20/11/17 21:09, Charles Baylis wrote:\n> On 15 September 2017 at 18:01, Kyrill Tkachov\n> <kyrylo.tkachov@foss.arm.com> wrote:\n> >\n> > On 15/09/17 16:38, Charles Baylis wrote:\n> >>\n> >> On 13 September 2017 at 10:02, Kyrill  Tkachov\n> >> <kyrylo.tkachov@foss.arm.com> wrote:\n> >>>\n> >>> Hi Charles,\n> >>>\n> >>> On 12/09/17 09:34, charles.baylis@linaro.org wrote:\n> >>>>\n> >>>> From: Charles Baylis <charles.baylis@linaro.org>\n> >>>>\n> >>>> This patch moves the calculation of costs for MEM into a\n> >>>> separate function, and reforms the calculation into two\n> >>>> parts. Firstly any additional cost of the addressing mode\n> >>>> is calculated, and then the cost of the memory access itself\n> >>>> is added.\n> >>>>\n> >>>> In this patch, the calculation of the cost of the addressing\n> >>>> mode is left as a placeholder, to be added in a subsequent\n> >>>> patch.\n> >>>>\n> >>> Can you please mention how has this series been tested?\n> >>> A bootstrap and test run on arm-none-linux-gnueabihf is required at\n> >>> least.\n> >>\n> >> It has been tested with make check on arm-unknown-linux-gnueabihf with\n> >> no regressions. I've successfully bootstrapped the next spin.\n> >\n> >\n> > Thanks.\n> >\n> >>> Also, do you have any benchmarking results for this?\n> >>> I agree that generating the addressing modes in the new tests is\n> >>> desirable.\n> >>> So I'm not objecting to the goal of this patch, but a check to \n> make sure\n> >>> that this doesn't regress SPEC\n> >>> would be great.  Further comments on the patch inline.\n> >>\n> >> SPEC2006 scores are unaffected by this patch on Cortex-A57.\n> >\n> >\n> > Good, thanks for checking :)\n> >\n> >\n> >>>> +/* Helper function for arm_rtx_costs_internal.  Calculates the \n> cost of\n> >>>> a\n> >>>> MEM,\n> >>>> +   considering the costs of the addressing mode and memory access\n> >>>> +   separately.  */\n> >>>> +static bool\n> >>>> +arm_mem_costs (rtx x, const struct cpu_cost_table *extra_cost,\n> >>>> +              int *cost, bool speed_p)\n> >>>> +{\n> >>>> +  machine_mode mode = GET_MODE (x);\n> >>>> +  if (flag_pic\n> >>>> +      && GET_CODE (XEXP (x, 0)) == PLUS\n> >>>> +      && will_be_in_index_register (XEXP (XEXP (x, 0), 1)))\n> >>>> +    /* This will be split into two instructions.  Add the cost \n> of the\n> >>>> +       additional instruction here.  The cost of the memory \n> access is\n> >>>> computed\n> >>>> +       below.  See arm.md:calculate_pic_address.  */\n> >>>> +    *cost = COSTS_N_INSNS (1);\n> >>>> +  else\n> >>>> +    *cost = 0;\n> >>>\n> >>>\n> >>> For speed_p we want the size cost of the insn (COSTS_N_INSNS (1) for a\n> >>> each\n> >>> insn)\n> >>> plus the appropriate field in extra_cost. So you should \n> unconditionally\n> >>> initialise the cost\n> >>> to COSTS_N_INSNS (1), conditionally increment it by COSTS_N_INSNS (1)\n> >>> with\n> >>> the condition above.\n> >>\n> >> OK. I also have to subtract that COSTS_N_INSNS (1) in the if (speed_p)\n> >> part because the cost of a single bus transfer is included in that\n> >> initial cost.\n> >>\n> >>>> +\n> >>>> +  /* Calculate cost of the addressing mode.  */\n> >>>> +  if (speed_p)\n> >>>> +    {\n> >>>> +      /* TODO: Add table-driven costs for addressing modes.  \n> (See patch\n> >>>> 2) */\n> >>>> +    }\n> >>>\n> >>>\n> >>> You mean \"patch 3\". I recommend you just remove this conditional from\n> >>> this\n> >>> patch and add the logic\n> >>> in patch 3 entirely.\n> >>\n> >> OK.\n> >>\n> >>>> +\n> >>>> +  /* Calculate cost of memory access.  */\n> >>>> +  if (speed_p)\n> >>>> +    {\n> >>>> +      /* data transfer is transfer size divided by bus width.  */\n> >>>> +      int bus_width_bytes = current_tune->bus_width / 4;\n> >>>\n> >>>\n> >>> This should be bus_width / BITS_PER_UNIT to get the size in bytes.\n> >>> BITS_PER_UNIT is 8 though, so you'll have to double check to make sure\n> >>> the\n> >>> cost calculation and generated code is still appropriate.\n> >>\n> >> Oops, I changed the units around and messed this up. I'll fix this.\n> >>\n> >>>> +      *cost += CEIL (GET_MODE_SIZE (mode), bus_width_bytes);\n> >>>> +      *cost += extra_cost->ldst.load;\n> >>>> +    }\n> >>>> +  else\n> >>>> +    {\n> >>>> +      *cost += COSTS_N_INSNS (1);\n> >>>> +    }\n> >>>\n> >>> Given my first comment above this else would be deleted.\n> >>\n> >> OK\n> >\n> >\n> > I have a concern about using the bus_width parameter which\n> > I explain in the thread for patch 1 (I don't think we need it, we \n> should use\n> > the fields in extra_cost->ldst\n> > more carefully).\n>\n> I have modified this patch accordingly. Patch 1 is no longer needed.\n>\n> Passes \"make check\" (with patch 3) on arm-linux-gnueabihf with no\n> regressions. Bootstrap is in progress.\n>\n> Can I still get this in during stage 3?\n>\n\nThanks, these are ok for trunk.\nThey were originally posted way before stage 3 and this is just a rework,\nso it's acceptable at this stage as far as I'm concerned.\n\nThank you for working on these,\nKyrill\n\n> gcc/ChangeLog:\n>\n> <date>  Charles Baylis <charles.baylis@linaro.org>\n>\n>         * config/arm/arm.c (arm_mem_costs): New function.\n>         (arm_rtx_costs_internal): Use arm_mem_costs.\n>\n> gcc/testsuite/ChangeLog:\n>\n> <date>  Charles Baylis <charles.baylis@linaro.org>\n>\n>         * gcc.target/arm/addr-modes-float.c: New test.\n>         * gcc.target/arm/addr-modes-int.c: New test.\n>         * gcc.target/arm/addr-modes.h: New header.","headers":{"Return-Path":"<gcc-patches-return-467771-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-467771-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"YcLCJRn6\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yjFGb6vdFz9sPm\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 23 Nov 2017 21:01:42 +1100 (AEDT)","(qmail 82674 invoked by alias); 23 Nov 2017 10:01:30 -0000","(qmail 82660 invoked by uid 89); 23 Nov 2017 10:01:29 -0000","from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by\n\tsourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tThu, 23 Nov 2017 10:01:19 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\tby\n\tusa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id\n\t894A180D; Thu, 23 Nov 2017 02:01:17 -0800 (PST)","from [10.2.207.77] (e100706-lin.cambridge.arm.com\n\t[10.2.207.77])\tby usa-sjc-imap-foss1.foss.arm.com (Postfix)\n\twith ESMTPSA id 988E53F487; Thu, 23 Nov 2017 02:01:16 -0800 (PST)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:date:from:mime-version:to:cc:subject:references\n\t:in-reply-to:content-type:content-transfer-encoding; q=dns; s=\n\tdefault; b=LBcGmdrqWRwCLHx774nzWGdHn8/k/jth0AU4icjonK3yAV9KOu1YJ\n\tkHORegg/eOUroRHfrwVfQXYUHkKqll3Bu5DwWbEFA8+IVUcq2NgKIFWvm+xZO/RR\n\tkPVYwcRczpODYEPNHkHrszvcoDKdv8IIoegCKhY6OC3a/0Vz5FDvQc=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:message-id:date:from:mime-version:to:cc:subject:references\n\t:in-reply-to:content-type:content-transfer-encoding; s=default;\n\tbh=qndPBoJtuQFqWD7we0sMX9abOhs=; b=YcLCJRn6guVUmiRrmHAc6/iQXq7l\n\tsWXoynyt+aalzOFAQUAuvblGnOTwfqF+A1fgdKjB+rq70Jz1RKZjhswuRR857fOO\n\txodZOuLVL2Hoi2vRYq3RMcsPUYT2pBfSKeQvnSVkeF4JJN/j0owdvjJWMOE3LOxL\n\tMLEfHhzK+TD3iy0=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-0.7 required=5.0 tests=BAYES_00,\n\tKAM_LAZY_DOMAIN_SECURITY, KB_WAM_FROM_NAME_SINGLEWORD,\n\tT_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=firstly,\n\tbenchmarking","X-HELO":"foss.arm.com","Message-ID":"<5A169C6B.8050509@foss.arm.com>","Date":"Thu, 23 Nov 2017 10:01:15 +0000","From":"Kyrill  Tkachov <kyrylo.tkachov@foss.arm.com>","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:31.0) Gecko/20100101 Thunderbird/31.2.0","MIME-Version":"1.0","To":"Charles Baylis <charles.baylis@linaro.org>","CC":"Richard Earnshaw <Richard.Earnshaw@arm.com>,\n\tRamana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,\n\t\"pinskia@gmail.com\" <pinskia@gmail.com>,\n\t\"gcc-patches@gcc.gnu.org\" <gcc-patches@gcc.gnu.org>","Subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","References":"<1505205277-26276-1-git-send-email-charles.baylis@linaro.org>\n\t<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>\n\t<59B8F43B.6090701@foss.arm.com>\n\t<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>\n\t<59BC0769.6080407@foss.arm.com>\n\t<CADnVucDOCv9j2_oSnde7-9fsqHw-at4WtSdNgdqmL8fxSyF-pg@mail.gmail.com>","In-Reply-To":"<CADnVucDOCv9j2_oSnde7-9fsqHw-at4WtSdNgdqmL8fxSyF-pg@mail.gmail.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit"}},{"id":1809452,"web_url":"http://patchwork.ozlabs.org/comment/1809452/","msgid":"<CADnVucDm1dh_XpW7v67WSO7NxWphNJmW-Fgcc4MnkmYKVP6yrg@mail.gmail.com>","list_archive_url":null,"date":"2017-11-23T18:53:33","subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","submitter":{"id":35578,"url":"http://patchwork.ozlabs.org/api/people/35578/","name":"Charles Baylis","email":"charles.baylis@linaro.org"},"content":"On 23 November 2017 at 10:01, Kyrill  Tkachov\n<kyrylo.tkachov@foss.arm.com> wrote:\n\n>\n> Thanks, these are ok for trunk.\n> They were originally posted way before stage 3 and this is just a rework,\n> so it's acceptable at this stage as far as I'm concerned.\n\nThanks. Committed to trunk as r255111.","headers":{"Return-Path":"<gcc-patches-return-467808-incoming=patchwork.ozlabs.org@gcc.gnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","mailing list gcc-patches@gcc.gnu.org"],"Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org\n\t(client-ip=209.132.180.131; helo=sourceware.org;\n\tenvelope-from=gcc-patches-return-467808-incoming=patchwork.ozlabs.org@gcc.gnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org\n\theader.b=\"O2cpwDM2\"; dkim-atps=neutral","sourceware.org; auth=none"],"Received":["from sourceware.org (server1.sourceware.org [209.132.180.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yjT4T6yYxz9s71\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 24 Nov 2017 05:53:44 +1100 (AEDT)","(qmail 92175 invoked by alias); 23 Nov 2017 18:53:37 -0000","(qmail 92165 invoked by uid 89); 23 Nov 2017 18:53:37 -0000","from mail-yb0-f171.google.com (HELO mail-yb0-f171.google.com)\n\t(209.85.213.171) by sourceware.org\n\t(qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP;\n\tThu, 23 Nov 2017 18:53:35 +0000","by mail-yb0-f171.google.com with SMTP id i126so7509134ybi.12 for\n\t<gcc-patches@gcc.gnu.org>; Thu, 23 Nov 2017 10:53:35 -0800 (PST)","by 10.129.182.1 with HTTP; Thu, 23 Nov 2017 10:53:33 -0800 (PST)"],"DomainKey-Signature":"a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; q=dns; s=default; b=APsqQ2SoOJ3p7Uc\n\tSc+R9FbDWbf9Ab6/BjQcXpnq7kOo79PG4DbD+PqAvV2+QOqjOpY1roTJSxieEvJT\n\tqZln8KSxq/25pJmHMaFVzB+dflcm6m1GHSGPxJv6HScRaG5DoH1IKmCi6KLORMlM\n\t2to8sQ/znzTDXBpfoR0XqDgbbwl0=","DKIM-Signature":"v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id\n\t:list-unsubscribe:list-archive:list-post:list-help:sender\n\t:mime-version:in-reply-to:references:from:date:message-id\n\t:subject:to:cc:content-type; s=default; bh=xczRp+zjMKxp54EIQTghz\n\tnNCEJs=; b=O2cpwDM2BZv1aEJXvzh9Ebb8Sa4q350t6tNTlFaWwvDhU3kFcEMHQ\n\tVvaUrS9TO4kWj7uYzZpgnseFCgHoBX3Ke5dVVlqDx7BE7E5h/jP6gfP692hCOiT3\n\t1ZLARv0KBddt2M2X6FegtQKbHm+vr7g8vllOCoSS1jJHSn6tj7y8mE=","Mailing-List":"contact gcc-patches-help@gcc.gnu.org; run by ezmlm","Precedence":"bulk","List-Id":"<gcc-patches.gcc.gnu.org>","List-Unsubscribe":"<mailto:gcc-patches-unsubscribe-incoming=patchwork.ozlabs.org@gcc.gnu.org>","List-Archive":"<http://gcc.gnu.org/ml/gcc-patches/>","List-Post":"<mailto:gcc-patches@gcc.gnu.org>","List-Help":"<mailto:gcc-patches-help@gcc.gnu.org>","Sender":"gcc-patches-owner@gcc.gnu.org","X-Virus-Found":"No","X-Spam-SWARE-Status":"No, score=-2.7 required=5.0 tests=AWL, BAYES_00,\n\tKB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_NONE,\n\tSPF_PASS autolearn=no version=3.3.2 spammy=","X-HELO":"mail-yb0-f171.google.com","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net;\n\ts=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=K1OJp+T7T/gAZN/bJ8wIdT3sxDQBimGktW5+UibmXKU=;\n\tb=nCGybE9bBHC9C1UgNkS58S5JwQpnPFdV1SqyeVwZ3QuMKvXel8k6/n9qaEDtSi/Y0b\n\tZ+2n67XAW/mRey3w8Zs/Xn7eVY1VjvzbXrJq/yStwr+N20OtfZcHiZmvfro1kqjaUaez\n\tG82qzinvi8G0HL/zBOHdnXwnEG01qKNTJala/VAzSPafj2kh5NaC5sPSMWBjg5NDD/c/\n\txrJeOdm3UL6DP4sf2ri6V9un4G5EyETlYPilbLaP1SiMZkF8nsDwQ+ks9mvalJWsF+RM\n\t2bgTtlYnnTrYHGqS4Jl5UB0mj6znuN2E0s+oEa9Al1gh37jntmJNkKy9aYNuhkLNCtec\n\tXnmA==","X-Gm-Message-State":"AJaThX72FJ2Q9RCrm/w+TI4bs+6IYye3Oyi30AUmrHTxJ/g7Dgu6z5wr\thxrnvBytcPnCrrEl0rocYm7rBhUZ/D8e0cVi66/oNQ==","X-Google-Smtp-Source":"AGs4zMY9PdVcrJF46jwbY5Fdy+WwLxwIN9wGs4vcT5h9LKRklGhYJApFRqsUog88RLeFcGvdc+TdYug0XQYwB9mUBvA=","X-Received":"by 10.37.188.210 with SMTP id l18mr16448061ybm.52.1511463213938;\n\tThu, 23 Nov 2017 10:53:33 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<5A169C6B.8050509@foss.arm.com>","References":"<1505205277-26276-1-git-send-email-charles.baylis@linaro.org>\n\t<1505205277-26276-3-git-send-email-charles.baylis@linaro.org>\n\t<59B8F43B.6090701@foss.arm.com>\n\t<CADnVucCXQuW0LW2zJ=3NDv2_yRns=QOEvktDEYMUeqh9xzowDw@mail.gmail.com>\n\t<59BC0769.6080407@foss.arm.com>\n\t<CADnVucDOCv9j2_oSnde7-9fsqHw-at4WtSdNgdqmL8fxSyF-pg@mail.gmail.com>\n\t<5A169C6B.8050509@foss.arm.com>","From":"Charles Baylis <charles.baylis@linaro.org>","Date":"Thu, 23 Nov 2017 18:53:33 +0000","Message-ID":"<CADnVucDm1dh_XpW7v67WSO7NxWphNJmW-Fgcc4MnkmYKVP6yrg@mail.gmail.com>","Subject":"Re: [PATCH 2/3] [ARM] Refactor costs calculation for MEM.","To":"Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>","Cc":"Richard Earnshaw <Richard.Earnshaw@arm.com>,\n\tRamana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,\n\t\"pinskia@gmail.com\" <pinskia@gmail.com>,\n\t\"gcc-patches@gcc.gnu.org\" <gcc-patches@gcc.gnu.org>","Content-Type":"text/plain; charset=\"UTF-8\"","X-IsSubscribed":"yes"}}]