[v3,18/42] intel/compiler: add a helper function to query hardware type table

Submitted by Iago Toral Quiroga on Jan. 15, 2019, 1:53 p.m.

Details

Message ID 20190115135414.2313-19-itoral@igalia.com
State New
Headers show
Series "intel: VK_KHR_shader_float16_int8 implementation" ( rev: 5 4 ) in Mesa

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga Jan. 15, 2019, 1:53 p.m.
We open coded this in a couple of places, so a helper function is probably
sensible. Plus it makes it more consistent with the 3src hardware type case.

Suggested-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
---
 src/intel/compiler/brw_reg_type.c | 34 ++++++++++++++++---------------
 1 file changed, 18 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/compiler/brw_reg_type.c b/src/intel/compiler/brw_reg_type.c
index 09b3ea61d4c..0c9f522eca0 100644
--- a/src/intel/compiler/brw_reg_type.c
+++ b/src/intel/compiler/brw_reg_type.c
@@ -193,6 +193,20 @@  static const struct hw_3src_type {
 #undef E
 };
 
+static inline const struct hw_type *
+get_hw_type_map(const struct gen_device_info *devinfo, uint32_t *size)
+{
+   if (devinfo->gen >= 11) {
+      if (size)
+         *size = ARRAY_SIZE(gen11_hw_type);
+      return gen11_hw_type;
+   } else {
+      if (size)
+         *size = ARRAY_SIZE(gen4_hw_type);
+      return gen4_hw_type;
+   }
+}
+
 /**
  * Convert a brw_reg_type enumeration value into the hardware representation.
  *
@@ -203,16 +217,10 @@  brw_reg_type_to_hw_type(const struct gen_device_info *devinfo,
                         enum brw_reg_file file,
                         enum brw_reg_type type)
 {
-   const struct hw_type *table;
-
-   if (devinfo->gen >= 11) {
-      assert(type < ARRAY_SIZE(gen11_hw_type));
-      table = gen11_hw_type;
-   } else {
-      assert(type < ARRAY_SIZE(gen4_hw_type));
-      table = gen4_hw_type;
-   }
+   uint32_t table_size;
+   const struct hw_type *table = get_hw_type_map(devinfo, &table_size);
 
+   assert(type < table_size);
    assert(devinfo->has_64bit_types || brw_reg_type_to_size(type) < 8 ||
           type == BRW_REGISTER_TYPE_NF);
 
@@ -234,13 +242,7 @@  enum brw_reg_type
 brw_hw_type_to_reg_type(const struct gen_device_info *devinfo,
                         enum brw_reg_file file, unsigned hw_type)
 {
-   const struct hw_type *table;
-
-   if (devinfo->gen >= 11) {
-      table = gen11_hw_type;
-   } else {
-      table = gen4_hw_type;
-   }
+   const struct hw_type *table = get_hw_type_map(devinfo, NULL);
 
    if (file == BRW_IMMEDIATE_VALUE) {
       for (enum brw_reg_type i = 0; i <= BRW_REGISTER_TYPE_LAST; i++) {

Comments

On Tue, Jan 15, 2019 at 7:54 AM Iago Toral Quiroga <itoral@igalia.com>
wrote:

> We open coded this in a couple of places, so a helper function is probably
> sensible. Plus it makes it more consistent with the 3src hardware type
> case.
>
> Suggested-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
> ---
>  src/intel/compiler/brw_reg_type.c | 34 ++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/src/intel/compiler/brw_reg_type.c
> b/src/intel/compiler/brw_reg_type.c
> index 09b3ea61d4c..0c9f522eca0 100644
> --- a/src/intel/compiler/brw_reg_type.c
> +++ b/src/intel/compiler/brw_reg_type.c
> @@ -193,6 +193,20 @@ static const struct hw_3src_type {
>  #undef E
>  };
>
> +static inline const struct hw_type *
> +get_hw_type_map(const struct gen_device_info *devinfo, uint32_t *size)
> +{
> +   if (devinfo->gen >= 11) {
> +      if (size)
> +         *size = ARRAY_SIZE(gen11_hw_type);
>

All of these type tables have the same size because we declare everything
through BRW_REGISTER_TYPE_LAST.  Do we really need to be returning the
array size separately for every table?


> +      return gen11_hw_type;
> +   } else {
> +      if (size)
> +         *size = ARRAY_SIZE(gen4_hw_type);
> +      return gen4_hw_type;
> +   }
> +}
> +
>  /**
>   * Convert a brw_reg_type enumeration value into the hardware
> representation.
>   *
> @@ -203,16 +217,10 @@ brw_reg_type_to_hw_type(const struct gen_device_info
> *devinfo,
>                          enum brw_reg_file file,
>                          enum brw_reg_type type)
>  {
> -   const struct hw_type *table;
> -
> -   if (devinfo->gen >= 11) {
> -      assert(type < ARRAY_SIZE(gen11_hw_type));
> -      table = gen11_hw_type;
> -   } else {
> -      assert(type < ARRAY_SIZE(gen4_hw_type));
> -      table = gen4_hw_type;
> -   }
> +   uint32_t table_size;
> +   const struct hw_type *table = get_hw_type_map(devinfo, &table_size);
>
> +   assert(type < table_size);
>     assert(devinfo->has_64bit_types || brw_reg_type_to_size(type) < 8 ||
>            type == BRW_REGISTER_TYPE_NF);
>
> @@ -234,13 +242,7 @@ enum brw_reg_type
>  brw_hw_type_to_reg_type(const struct gen_device_info *devinfo,
>                          enum brw_reg_file file, unsigned hw_type)
>  {
> -   const struct hw_type *table;
> -
> -   if (devinfo->gen >= 11) {
> -      table = gen11_hw_type;
> -   } else {
> -      table = gen4_hw_type;
> -   }
> +   const struct hw_type *table = get_hw_type_map(devinfo, NULL);
>
>     if (file == BRW_IMMEDIATE_VALUE) {
>        for (enum brw_reg_type i = 0; i <= BRW_REGISTER_TYPE_LAST; i++) {
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
On Thu, 2019-01-17 at 14:16 -0600, Jason Ekstrand wrote:
> On Tue, Jan 15, 2019 at 7:54 AM Iago Toral Quiroga <itoral@igalia.com
> > wrote:
> > We open coded this in a couple of places, so a helper function is
> > probably
> > 
> > sensible. Plus it makes it more consistent with the 3src hardware
> > type case.
> > 
> > 
> > 
> > Suggested-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
> > 
> > ---
> > 
> >  src/intel/compiler/brw_reg_type.c | 34 ++++++++++++++++-----------
> > ----
> > 
> >  1 file changed, 18 insertions(+), 16 deletions(-)
> > 
> > 
> > 
> > diff --git a/src/intel/compiler/brw_reg_type.c
> > b/src/intel/compiler/brw_reg_type.c
> > 
> > index 09b3ea61d4c..0c9f522eca0 100644
> > 
> > --- a/src/intel/compiler/brw_reg_type.c
> > 
> > +++ b/src/intel/compiler/brw_reg_type.c
> > 
> > @@ -193,6 +193,20 @@ static const struct hw_3src_type {
> > 
> >  #undef E
> > 
> >  };
> > 
> > 
> > 
> > +static inline const struct hw_type *
> > 
> > +get_hw_type_map(const struct gen_device_info *devinfo, uint32_t
> > *size)
> > 
> > +{
> > 
> > +   if (devinfo->gen >= 11) {
> > 
> > +      if (size)
> > 
> > +         *size = ARRAY_SIZE(gen11_hw_type);
> 
> All of these type tables have the same size because we declare
> everything through BRW_REGISTER_TYPE_LAST.  Do we really need to be
> returning the array size separately for every table?

Good point, I guess we can drop that in this patch and the previous and
simply check that whatever type we are indexing is <=
BRW_REGISTER_TYPE_LAST. I'll do that.
> > +      return gen11_hw_type;
> > 
> > +   } else {
> > 
> > +      if (size)
> > 
> > +         *size = ARRAY_SIZE(gen4_hw_type);
> > 
> > +      return gen4_hw_type;
> > 
> > +   }
> > 
> > +}
> > 
> > +
> > 
> >  /**
> > 
> >   * Convert a brw_reg_type enumeration value into the hardware
> > representation.
> > 
> >   *
> > 
> > @@ -203,16 +217,10 @@ brw_reg_type_to_hw_type(const struct
> > gen_device_info *devinfo,
> > 
> >                          enum brw_reg_file file,
> > 
> >                          enum brw_reg_type type)
> > 
> >  {
> > 
> > -   const struct hw_type *table;
> > 
> > -
> > 
> > -   if (devinfo->gen >= 11) {
> > 
> > -      assert(type < ARRAY_SIZE(gen11_hw_type));
> > 
> > -      table = gen11_hw_type;
> > 
> > -   } else {
> > 
> > -      assert(type < ARRAY_SIZE(gen4_hw_type));
> > 
> > -      table = gen4_hw_type;
> > 
> > -   }
> > 
> > +   uint32_t table_size;
> > 
> > +   const struct hw_type *table = get_hw_type_map(devinfo,
> > &table_size);
> > 
> > 
> > 
> > +   assert(type < table_size);
> > 
> >     assert(devinfo->has_64bit_types || brw_reg_type_to_size(type) <
> > 8 ||
> > 
> >            type == BRW_REGISTER_TYPE_NF);
> > 
> > 
> > 
> > @@ -234,13 +242,7 @@ enum brw_reg_type
> > 
> >  brw_hw_type_to_reg_type(const struct gen_device_info *devinfo,
> > 
> >                          enum brw_reg_file file, unsigned hw_type)
> > 
> >  {
> > 
> > -   const struct hw_type *table;
> > 
> > -
> > 
> > -   if (devinfo->gen >= 11) {
> > 
> > -      table = gen11_hw_type;
> > 
> > -   } else {
> > 
> > -      table = gen4_hw_type;
> > 
> > -   }
> > 
> > +   const struct hw_type *table = get_hw_type_map(devinfo, NULL);
> > 
> > 
> > 
> >     if (file == BRW_IMMEDIATE_VALUE) {
> > 
> >        for (enum brw_reg_type i = 0; i <= BRW_REGISTER_TYPE_LAST;
> > i++) {
> >