diff mbox series

arm: Initialize vector costing fields

Message ID	20211108181316.3193199-1-christophe.lyon@foss.st.com
State	Committed
Commit	1200e211a823816e47a9312efab61a60e12e33e5
Headers	DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1F508385802A To: <gcc-patches@gcc.gnu.org> Subject: [PATCH] arm: Initialize vector costing fields Date: Mon, 8 Nov 2021 19:12:56 +0100 Message-ID: <20211108181316.3193199-1-christophe.lyon@foss.st.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain Precedence: list From: Christophe Lyon via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Christophe Lyon <christophe.lyon@foss.st.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org>
Series	arm: Initialize vector costing fields \| arm: Initialize vector costing fields

Commit Message

Christophe Lyon Nov. 8, 2021, 6:12 p.m. UTC

  The movi, dup and extract costing fields were recently added to struct
vector_cost_table, but there initialization is missing for the arm
(aarch32) specific descriptions.

Although the arm port does not use these fields (only aarch64 does),
this is causing warnings during the build, and even build failures
when using gcc-4.8.5 as host compiler:

/gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member 'vector_cost_table::movi'
 };
  ^
/gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for member 'vector_cost_table::movi' [-Wmissing-field-initializers]
/gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member 'vector_cost_table::dup'
/gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for member 'vector_cost_table::dup' [-Wmissing-field-initializers]
/gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member 'vector_cost_table::extract'
/gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for member 'vector_cost_table::extract' [-Wmissing-field-initializers]

This patch uses the same initialization values as in aarch64 for
consistency:
+    COSTS_N_INSNS (1),  /* movi.  */
+    COSTS_N_INSNS (2),  /* dup.  */
+    COSTS_N_INSNS (2)   /* extract.  */

But given these fields are not used, maybe a dummy value should be
used instead? (zero?)

2021-11-08  Christophe Lyon  <christophe.lyon@foss.st.com>

	gcc/
	* config/arm/arm.c (cortexa9_extra_costs, cortexa8_extra_costs,
	cortexa5_extra_costs, cortexa7_extra_costs,
	cortexa12_extra_costs, cortexa15_extra_costs, v7m_extra_costs):
	Initialize movi, dup and extract costing fields.
---
 gcc/config/arm/arm.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

Comments

Kyrylo Tkachov Nov. 10, 2021, 3:34 p.m. UTC | #1

Hi Christophe

> -----Original Message-----
> From: Gcc-patches <gcc-patches-
> bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Christophe
> Lyon via Gcc-patches
> Sent: Monday, November 8, 2021 6:13 PM
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH] arm: Initialize vector costing fields
> 
> The movi, dup and extract costing fields were recently added to struct
> vector_cost_table, but there initialization is missing for the arm
> (aarch32) specific descriptions.
> 
> Although the arm port does not use these fields (only aarch64 does),
> this is causing warnings during the build, and even build failures
> when using gcc-4.8.5 as host compiler:
> 
> /gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member
> 'vector_cost_table::movi'
>  };
>   ^
> /gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for member
> 'vector_cost_table::movi' [-Wmissing-field-initializers]
> /gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member
> 'vector_cost_table::dup'
> /gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for member
> 'vector_cost_table::dup' [-Wmissing-field-initializers]
> /gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member
> 'vector_cost_table::extract'
> /gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for member
> 'vector_cost_table::extract' [-Wmissing-field-initializers]
> 
> This patch uses the same initialization values as in aarch64 for
> consistency:
> +    COSTS_N_INSNS (1),  /* movi.  */
> +    COSTS_N_INSNS (2),  /* dup.  */
> +    COSTS_N_INSNS (2)   /* extract.  */
> 
> But given these fields are not used, maybe a dummy value should be
> used instead? (zero?)

They're dummy values for now, but there's no reason why the backend couldn't be extended to use them in the future.
Anyway, this patch is okay as is.

Thanks,
Kyrill

> 
> 2021-11-08  Christophe Lyon  <christophe.lyon@foss.st.com>
> 
> 	gcc/
> 	* config/arm/arm.c (cortexa9_extra_costs, cortexa8_extra_costs,
> 	cortexa5_extra_costs, cortexa7_extra_costs,
> 	cortexa12_extra_costs, cortexa15_extra_costs, v7m_extra_costs):
> 	Initialize movi, dup and extract costing fields.
> ---
>  gcc/config/arm/arm.c | 35 ++++++++++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 6c6e77fab66..3f5e1162853 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -1197,7 +1197,10 @@ const struct cpu_cost_table cortexa9_extra_costs
> =
>    /* Vector */
>    {
>      COSTS_N_INSNS (1),	/* alu.  */
> -    COSTS_N_INSNS (4)	/* mult.  */
> +    COSTS_N_INSNS (4),	/* mult.  */
> +    COSTS_N_INSNS (1),	/* movi.  */
> +    COSTS_N_INSNS (2),	/* dup.  */
> +    COSTS_N_INSNS (2)	/* extract.  */
>    }
>  };
> 
> @@ -1301,7 +1304,10 @@ const struct cpu_cost_table cortexa8_extra_costs
> =
>    /* Vector */
>    {
>      COSTS_N_INSNS (1),	/* alu.  */
> -    COSTS_N_INSNS (4)	/* mult.  */
> +    COSTS_N_INSNS (4),	/* mult.  */
> +    COSTS_N_INSNS (1),	/* movi.  */
> +    COSTS_N_INSNS (2),	/* dup.  */
> +    COSTS_N_INSNS (2)	/* extract.  */
>    }
>  };
> 
> @@ -1406,7 +1412,10 @@ const struct cpu_cost_table cortexa5_extra_costs
> =
>    /* Vector */
>    {
>      COSTS_N_INSNS (1),	/* alu.  */
> -    COSTS_N_INSNS (4)	/* mult.  */
> +    COSTS_N_INSNS (4),	/* mult.  */
> +    COSTS_N_INSNS (1),	/* movi.  */
> +    COSTS_N_INSNS (2),	/* dup.  */
> +    COSTS_N_INSNS (2)	/* extract.  */
>    }
>  };
> 
> @@ -1512,7 +1521,10 @@ const struct cpu_cost_table cortexa7_extra_costs
> =
>    /* Vector */
>    {
>      COSTS_N_INSNS (1),	/* alu.  */
> -    COSTS_N_INSNS (4)	/* mult.  */
> +    COSTS_N_INSNS (4),	/* mult.  */
> +    COSTS_N_INSNS (1),	/* movi.  */
> +    COSTS_N_INSNS (2),	/* dup.  */
> +    COSTS_N_INSNS (2)	/* extract.  */
>    }
>  };
> 
> @@ -1616,7 +1628,10 @@ const struct cpu_cost_table
> cortexa12_extra_costs =
>    /* Vector */
>    {
>      COSTS_N_INSNS (1),	/* alu.  */
> -    COSTS_N_INSNS (4)	/* mult.  */
> +    COSTS_N_INSNS (4),	/* mult.  */
> +    COSTS_N_INSNS (1),	/* movi.  */
> +    COSTS_N_INSNS (2),	/* dup.  */
> +    COSTS_N_INSNS (2)	/* extract.  */
>    }
>  };
> 
> @@ -1720,7 +1735,10 @@ const struct cpu_cost_table
> cortexa15_extra_costs =
>    /* Vector */
>    {
>      COSTS_N_INSNS (1),	/* alu.  */
> -    COSTS_N_INSNS (4)	/* mult.  */
> +    COSTS_N_INSNS (4),	/* mult.  */
> +    COSTS_N_INSNS (1),	/* movi.  */
> +    COSTS_N_INSNS (2),	/* dup.  */
> +    COSTS_N_INSNS (2)	/* extract.  */
>    }
>  };
> 
> @@ -1824,7 +1842,10 @@ const struct cpu_cost_table v7m_extra_costs =
>    /* Vector */
>    {
>      COSTS_N_INSNS (1),	/* alu.  */
> -    COSTS_N_INSNS (4)	/* mult.  */
> +    COSTS_N_INSNS (4),	/* mult.  */
> +    COSTS_N_INSNS (1),	/* movi.  */
> +    COSTS_N_INSNS (2),	/* dup.  */
> +    COSTS_N_INSNS (2)	/* extract.  */
>    }
>  };
> 
> --
> 2.25.1

Christophe Lyon Nov. 10, 2021, 5 p.m. UTC | #2

On Wed, Nov 10, 2021 at 4:34 PM Kyrylo Tkachov via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

> Hi Christophe
>
> > -----Original Message-----
> > From: Gcc-patches <gcc-patches-
> > bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Christophe
> > Lyon via Gcc-patches
> > Sent: Monday, November 8, 2021 6:13 PM
> > To: gcc-patches@gcc.gnu.org
> > Subject: [PATCH] arm: Initialize vector costing fields
> >
> > The movi, dup and extract costing fields were recently added to struct
> > vector_cost_table, but there initialization is missing for the arm
> > (aarch32) specific descriptions.
> >
> > Although the arm port does not use these fields (only aarch64 does),
> > this is causing warnings during the build, and even build failures
> > when using gcc-4.8.5 as host compiler:
> >
> > /gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member
> > 'vector_cost_table::movi'
> >  };
> >   ^
> > /gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for
> member
> > 'vector_cost_table::movi' [-Wmissing-field-initializers]
> > /gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member
> > 'vector_cost_table::dup'
> > /gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for
> member
> > 'vector_cost_table::dup' [-Wmissing-field-initializers]
> > /gccsrc/gcc/config/arm/arm.c:1194:1: error: uninitialized const member
> > 'vector_cost_table::extract'
> > /gccsrc/gcc/config/arm/arm.c:1194:1: warning: missing initializer for
> member
> > 'vector_cost_table::extract' [-Wmissing-field-initializers]
> >
> > This patch uses the same initialization values as in aarch64 for
> > consistency:
> > +    COSTS_N_INSNS (1),  /* movi.  */
> > +    COSTS_N_INSNS (2),  /* dup.  */
> > +    COSTS_N_INSNS (2)   /* extract.  */
> >
> > But given these fields are not used, maybe a dummy value should be
> > used instead? (zero?)
>
> They're dummy values for now, but there's no reason why the backend
> couldn't be extended to use them in the future.
> Anyway, this patch is okay as is.
>
>
Thanks, pushed as  r12-5132-g1200e211a823816e47a9312efab61a60e12e33e5

Christophe

Thanks,
> Kyrill
>
> >
> > 2021-11-08  Christophe Lyon  <christophe.lyon@foss.st.com>
> >
> >       gcc/
> >       * config/arm/arm.c (cortexa9_extra_costs, cortexa8_extra_costs,
> >       cortexa5_extra_costs, cortexa7_extra_costs,
> >       cortexa12_extra_costs, cortexa15_extra_costs, v7m_extra_costs):
> >       Initialize movi, dup and extract costing fields.
> > ---
> >  gcc/config/arm/arm.c | 35 ++++++++++++++++++++++++++++-------
> >  1 file changed, 28 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> > index 6c6e77fab66..3f5e1162853 100644
> > --- a/gcc/config/arm/arm.c
> > +++ b/gcc/config/arm/arm.c
> > @@ -1197,7 +1197,10 @@ const struct cpu_cost_table cortexa9_extra_costs
> > =
> >    /* Vector */
> >    {
> >      COSTS_N_INSNS (1),       /* alu.  */
> > -    COSTS_N_INSNS (4)        /* mult.  */
> > +    COSTS_N_INSNS (4),       /* mult.  */
> > +    COSTS_N_INSNS (1),       /* movi.  */
> > +    COSTS_N_INSNS (2),       /* dup.  */
> > +    COSTS_N_INSNS (2)        /* extract.  */
> >    }
> >  };
> >
> > @@ -1301,7 +1304,10 @@ const struct cpu_cost_table cortexa8_extra_costs
> > =
> >    /* Vector */
> >    {
> >      COSTS_N_INSNS (1),       /* alu.  */
> > -    COSTS_N_INSNS (4)        /* mult.  */
> > +    COSTS_N_INSNS (4),       /* mult.  */
> > +    COSTS_N_INSNS (1),       /* movi.  */
> > +    COSTS_N_INSNS (2),       /* dup.  */
> > +    COSTS_N_INSNS (2)        /* extract.  */
> >    }
> >  };
> >
> > @@ -1406,7 +1412,10 @@ const struct cpu_cost_table cortexa5_extra_costs
> > =
> >    /* Vector */
> >    {
> >      COSTS_N_INSNS (1),       /* alu.  */
> > -    COSTS_N_INSNS (4)        /* mult.  */
> > +    COSTS_N_INSNS (4),       /* mult.  */
> > +    COSTS_N_INSNS (1),       /* movi.  */
> > +    COSTS_N_INSNS (2),       /* dup.  */
> > +    COSTS_N_INSNS (2)        /* extract.  */
> >    }
> >  };
> >
> > @@ -1512,7 +1521,10 @@ const struct cpu_cost_table cortexa7_extra_costs
> > =
> >    /* Vector */
> >    {
> >      COSTS_N_INSNS (1),       /* alu.  */
> > -    COSTS_N_INSNS (4)        /* mult.  */
> > +    COSTS_N_INSNS (4),       /* mult.  */
> > +    COSTS_N_INSNS (1),       /* movi.  */
> > +    COSTS_N_INSNS (2),       /* dup.  */
> > +    COSTS_N_INSNS (2)        /* extract.  */
> >    }
> >  };
> >
> > @@ -1616,7 +1628,10 @@ const struct cpu_cost_table
> > cortexa12_extra_costs =
> >    /* Vector */
> >    {
> >      COSTS_N_INSNS (1),       /* alu.  */
> > -    COSTS_N_INSNS (4)        /* mult.  */
> > +    COSTS_N_INSNS (4),       /* mult.  */
> > +    COSTS_N_INSNS (1),       /* movi.  */
> > +    COSTS_N_INSNS (2),       /* dup.  */
> > +    COSTS_N_INSNS (2)        /* extract.  */
> >    }
> >  };
> >
> > @@ -1720,7 +1735,10 @@ const struct cpu_cost_table
> > cortexa15_extra_costs =
> >    /* Vector */
> >    {
> >      COSTS_N_INSNS (1),       /* alu.  */
> > -    COSTS_N_INSNS (4)        /* mult.  */
> > +    COSTS_N_INSNS (4),       /* mult.  */
> > +    COSTS_N_INSNS (1),       /* movi.  */
> > +    COSTS_N_INSNS (2),       /* dup.  */
> > +    COSTS_N_INSNS (2)        /* extract.  */
> >    }
> >  };
> >
> > @@ -1824,7 +1842,10 @@ const struct cpu_cost_table v7m_extra_costs =
> >    /* Vector */
> >    {
> >      COSTS_N_INSNS (1),       /* alu.  */
> > -    COSTS_N_INSNS (4)        /* mult.  */
> > +    COSTS_N_INSNS (4),       /* mult.  */
> > +    COSTS_N_INSNS (1),       /* movi.  */
> > +    COSTS_N_INSNS (2),       /* dup.  */
> > +    COSTS_N_INSNS (2)        /* extract.  */
> >    }
> >  };
> >
> > --
> > 2.25.1
>
>

diff mbox series

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6c6e77fab66..3f5e1162853 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -1197,7 +1197,10 @@  const struct cpu_cost_table cortexa9_extra_costs =
   /* Vector */
   {
     COSTS_N_INSNS (1),	/* alu.  */
-    COSTS_N_INSNS (4)	/* mult.  */
+    COSTS_N_INSNS (4),	/* mult.  */
+    COSTS_N_INSNS (1),	/* movi.  */
+    COSTS_N_INSNS (2),	/* dup.  */
+    COSTS_N_INSNS (2)	/* extract.  */
   }
 };
 
@@ -1301,7 +1304,10 @@  const struct cpu_cost_table cortexa8_extra_costs =
   /* Vector */
   {
     COSTS_N_INSNS (1),	/* alu.  */
-    COSTS_N_INSNS (4)	/* mult.  */
+    COSTS_N_INSNS (4),	/* mult.  */
+    COSTS_N_INSNS (1),	/* movi.  */
+    COSTS_N_INSNS (2),	/* dup.  */
+    COSTS_N_INSNS (2)	/* extract.  */
   }
 };
 
@@ -1406,7 +1412,10 @@  const struct cpu_cost_table cortexa5_extra_costs =
   /* Vector */
   {
     COSTS_N_INSNS (1),	/* alu.  */
-    COSTS_N_INSNS (4)	/* mult.  */
+    COSTS_N_INSNS (4),	/* mult.  */
+    COSTS_N_INSNS (1),	/* movi.  */
+    COSTS_N_INSNS (2),	/* dup.  */
+    COSTS_N_INSNS (2)	/* extract.  */
   }
 };
 
@@ -1512,7 +1521,10 @@  const struct cpu_cost_table cortexa7_extra_costs =
   /* Vector */
   {
     COSTS_N_INSNS (1),	/* alu.  */
-    COSTS_N_INSNS (4)	/* mult.  */
+    COSTS_N_INSNS (4),	/* mult.  */
+    COSTS_N_INSNS (1),	/* movi.  */
+    COSTS_N_INSNS (2),	/* dup.  */
+    COSTS_N_INSNS (2)	/* extract.  */
   }
 };
 
@@ -1616,7 +1628,10 @@  const struct cpu_cost_table cortexa12_extra_costs =
   /* Vector */
   {
     COSTS_N_INSNS (1),	/* alu.  */
-    COSTS_N_INSNS (4)	/* mult.  */
+    COSTS_N_INSNS (4),	/* mult.  */
+    COSTS_N_INSNS (1),	/* movi.  */
+    COSTS_N_INSNS (2),	/* dup.  */
+    COSTS_N_INSNS (2)	/* extract.  */
   }
 };
 
@@ -1720,7 +1735,10 @@  const struct cpu_cost_table cortexa15_extra_costs =
   /* Vector */
   {
     COSTS_N_INSNS (1),	/* alu.  */
-    COSTS_N_INSNS (4)	/* mult.  */
+    COSTS_N_INSNS (4),	/* mult.  */
+    COSTS_N_INSNS (1),	/* movi.  */
+    COSTS_N_INSNS (2),	/* dup.  */
+    COSTS_N_INSNS (2)	/* extract.  */
   }
 };
 
@@ -1824,7 +1842,10 @@  const struct cpu_cost_table v7m_extra_costs =
   /* Vector */
   {
     COSTS_N_INSNS (1),	/* alu.  */
-    COSTS_N_INSNS (4)	/* mult.  */
+    COSTS_N_INSNS (4),	/* mult.  */
+    COSTS_N_INSNS (1),	/* movi.  */
+    COSTS_N_INSNS (2),	/* dup.  */
+    COSTS_N_INSNS (2)	/* extract.  */
   }
 };