public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] arm: Initialize vector costing fields
@ 2021-11-08 18:12 Christophe Lyon
  2021-11-10 15:34 ` Kyrylo Tkachov
  0 siblings, 1 reply; 3+ messages in thread
From: Christophe Lyon @ 2021-11-08 18:12 UTC (permalink / raw)
  To: gcc-patches

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(-)

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] arm: Initialize vector costing fields
  2021-11-08 18:12 [PATCH] arm: Initialize vector costing fields Christophe Lyon
@ 2021-11-10 15:34 ` Kyrylo Tkachov
  2021-11-10 17:00   ` Christophe Lyon
  0 siblings, 1 reply; 3+ messages in thread
From: Kyrylo Tkachov @ 2021-11-10 15:34 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: gcc-patches

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm: Initialize vector costing fields
  2021-11-10 15:34 ` Kyrylo Tkachov
@ 2021-11-10 17:00   ` Christophe Lyon
  0 siblings, 0 replies; 3+ messages in thread
From: Christophe Lyon @ 2021-11-10 17:00 UTC (permalink / raw)
  To: Kyrylo Tkachov; +Cc: Christophe Lyon, gcc-patches, Tamar Christina

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
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-10 17:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-08 18:12 [PATCH] arm: Initialize vector costing fields Christophe Lyon
2021-11-10 15:34 ` Kyrylo Tkachov
2021-11-10 17:00   ` Christophe Lyon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).