Re: [Mesa-dev] [PATCH 12/47] i965/fs: Add brw_reg_type_from_bit_size utility method

[Chema Casanova]

El 26/08/17 a las 19:19, Jason Ekstrand escribió:
> On Thu, Aug 24, 2017 at 6:54 AM, Alejandro Piñeiro
> mailto:apinhe...@igalia.com>> wrote:
>
> Returns the brw_type for a given ssa.bit_size, and a reference type.
> So if bit_size is 64, and the reference type is BRW_REGISTER_TYPE_F,
> it returns BRW_REGISTER_TYPE_DF. The same applies if bit_size is 32
> and reference type is BRW_REGISTER_TYPE_HF it returns
> BRW_REGISTER_TYPE_F
>
> Signed-off-by: Jose Maria Casanova Crespo  >
> Signed-off-by: Alejandro Piñeiro  
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 67
> ---
>  1 file changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index d760946e624..e4eba1401f8 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -214,6 +214,63 @@ fs_visitor::nir_emit_system_values()
>     }
>  }
>
> +/*
> + * Returns a type based on a reference_type (word, float,
> half-float) and a
> + * given bit_size.
> + *
> + * Reference BRW_REGISTER_TYPE are HF,F,DF,W,D,UW,UD.
> + *
> + * @FIXME: 64-bit return types are always DF on integer types to
> maintain
> + * compability with uses of DF previously to the introduction of
> int64
> + * support.
> + */
> +static brw_reg_type
> +brw_reg_type_from_bit_size(const unsigned bit_size,
> +                           const brw_reg_type reference_type)
> +{
> +   switch(reference_type) {
> +   case BRW_REGISTER_TYPE_HF:
> +   case BRW_REGISTER_TYPE_F:
> +   case BRW_REGISTER_TYPE_DF:
> +      switch(bit_size) {
> +      case 16:
> +         return BRW_REGISTER_TYPE_HF;
> +      case 32:
> +         return BRW_REGISTER_TYPE_F;
> +      case 64:
> +         return BRW_REGISTER_TYPE_DF;
> +      default:
> +         unreachable("Not reached");
>
>
> Please add something more descriptive here such as "Invalid bit size"
OK.
>  
>
> +      }
> +   case BRW_REGISTER_TYPE_W:
> +   case BRW_REGISTER_TYPE_D:
>
>
> Please add the Q type
OK.
>  
>
> +      switch(bit_size) {
> +      case 16:
> +         return BRW_REGISTER_TYPE_W;
> +      case 32:
> +         return BRW_REGISTER_TYPE_D;
> +      case 64:
> +         return BRW_REGISTER_TYPE_DF;
> +      default:
> +         unreachable("Not reached");
>
>
> Better message
OK.
>  
>
> +      }
> +   case BRW_REGISTER_TYPE_UW:
> +   case BRW_REGISTER_TYPE_UD:
>
>
> Please add the UQ type
OK.
>  
>
> +      switch(bit_size) {
> +      case 16:
> +         return BRW_REGISTER_TYPE_UW;
> +      case 32:
> +         return BRW_REGISTER_TYPE_UD;
> +      case 64:
> +         return BRW_REGISTER_TYPE_DF;
> +      default:
> +         unreachable("Not reached");
>
>
> better message
>  
>
> +      }
> +   default:
> +      unreachable("Not reached");
>
>
> better message
>
> I've got all those fixes in a version of this patch I pulled into my
> subgroups tree.
>  
So I finally picked locally your review of this patch from:
https://cgit.freedesktop.org/~jekstrand/mesa/commit/?id=c21aee439ffc15a7b8cec811727c0efb5e2bfa6c

I didn't find your subgroups tree.

Thanks for the review.

Chema
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 12/47] i965/fs: Add brw_reg_type_from_bit_size utility method

[Jason Ekstrand]

On Thu, Aug 24, 2017 at 6:54 AM, Alejandro Piñeiro 
wrote:

> Returns the brw_type for a given ssa.bit_size, and a reference type.
> So if bit_size is 64, and the reference type is BRW_REGISTER_TYPE_F,
> it returns BRW_REGISTER_TYPE_DF. The same applies if bit_size is 32
> and reference type is BRW_REGISTER_TYPE_HF it returns BRW_REGISTER_TYPE_F
>
> Signed-off-by: Jose Maria Casanova Crespo 
> Signed-off-by: Alejandro Piñeiro  ---
>  src/intel/compiler/brw_fs_nir.cpp | 67 ++
> ++---
>  1 file changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index d760946e624..e4eba1401f8 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -214,6 +214,63 @@ fs_visitor::nir_emit_system_values()
> }
>  }
>
> +/*
> + * Returns a type based on a reference_type (word, float, half-float) and
> a
> + * given bit_size.
> + *
> + * Reference BRW_REGISTER_TYPE are HF,F,DF,W,D,UW,UD.
> + *
> + * @FIXME: 64-bit return types are always DF on integer types to maintain
> + * compability with uses of DF previously to the introduction of int64
> + * support.
> + */
> +static brw_reg_type
> +brw_reg_type_from_bit_size(const unsigned bit_size,
> +   const brw_reg_type reference_type)
> +{
> +   switch(reference_type) {
> +   case BRW_REGISTER_TYPE_HF:
> +   case BRW_REGISTER_TYPE_F:
> +   case BRW_REGISTER_TYPE_DF:
> +  switch(bit_size) {
> +  case 16:
> + return BRW_REGISTER_TYPE_HF;
> +  case 32:
> + return BRW_REGISTER_TYPE_F;
> +  case 64:
> + return BRW_REGISTER_TYPE_DF;
> +  default:
> + unreachable("Not reached");
>

Please add something more descriptive here such as "Invalid bit size"


> +  }
> +   case BRW_REGISTER_TYPE_W:
> +   case BRW_REGISTER_TYPE_D:
>

Please add the Q type


> +  switch(bit_size) {
> +  case 16:
> + return BRW_REGISTER_TYPE_W;
> +  case 32:
> + return BRW_REGISTER_TYPE_D;
> +  case 64:
> + return BRW_REGISTER_TYPE_DF;
> +  default:
> + unreachable("Not reached");
>

Better message


> +  }
> +   case BRW_REGISTER_TYPE_UW:
> +   case BRW_REGISTER_TYPE_UD:
>

Please add the UQ type


> +  switch(bit_size) {
> +  case 16:
> + return BRW_REGISTER_TYPE_UW;
> +  case 32:
> + return BRW_REGISTER_TYPE_UD;
> +  case 64:
> + return BRW_REGISTER_TYPE_DF;
> +  default:
> + unreachable("Not reached");
>

better message


> +  }
> +   default:
> +  unreachable("Not reached");
>

better message

I've got all those fixes in a version of this patch I pulled into my
subgroups tree.


> +   }
> +}
> +
>  void
>  fs_visitor::nir_emit_impl(nir_function_impl *impl)
>  {
> @@ -227,7 +284,7 @@ fs_visitor::nir_emit_impl(nir_function_impl *impl)
>   reg->num_array_elems == 0 ? 1 : reg->num_array_elems;
>unsigned size = array_elems * reg->num_components;
>const brw_reg_type reg_type =
> - reg->bit_size == 32 ? BRW_REGISTER_TYPE_F : BRW_REGISTER_TYPE_DF;
> + brw_reg_type_from_bit_size(reg->bit_size, BRW_REGISTER_TYPE_F);
>nir_locals[reg->index] = bld.vgrf(reg_type, size);
> }
>
> @@ -1338,7 +1395,7 @@ fs_visitor::nir_emit_load_const(const fs_builder
> &bld,
>  nir_load_const_instr *instr)
>  {
> const brw_reg_type reg_type =
> -  instr->def.bit_size == 32 ? BRW_REGISTER_TYPE_D :
> BRW_REGISTER_TYPE_DF;
> +  brw_reg_type_from_bit_size(instr->def.bit_size,
> BRW_REGISTER_TYPE_D);
> fs_reg reg = bld.vgrf(reg_type, instr->def.num_components);
>
> switch (instr->def.bit_size) {
> @@ -1366,8 +1423,8 @@ fs_visitor::get_nir_src(const nir_src &src)
> fs_reg reg;
> if (src.is_ssa) {
>if (src.ssa->parent_instr->type == nir_instr_type_ssa_undef) {
> - const brw_reg_type reg_type = src.ssa->bit_size == 32 ?
> -BRW_REGISTER_TYPE_D : BRW_REGISTER_TYPE_DF;
> + const brw_reg_type reg_type =
> +brw_reg_type_from_bit_size(src.ssa->bit_size,
> BRW_REGISTER_TYPE_D);
>   reg = bld.vgrf(reg_type, src.ssa->num_components);
>} else {
>   reg = nir_ssa_values[src.ssa->index];
> @@ -1401,7 +1458,7 @@ fs_visitor::get_nir_dest(const nir_dest &dest)
>  {
> if (dest.is_ssa) {
>const brw_reg_type reg_type =
> - dest.ssa.bit_size == 32 ? BRW_REGISTER_TYPE_F :
> BRW_REGISTER_TYPE_DF;
> + brw_reg_type_from_bit_size(dest.ssa.bit_size,
> BRW_REGISTER_TYPE_F);
>nir_ssa_values[dest.ssa.index] =
>   bld.vgrf(reg_type, dest.ssa.num_components);
>return nir_ssa_values[dest.ssa.index];
> --
> 2.11.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
__

[Mesa-dev] [PATCH 12/47] i965/fs: Add brw_reg_type_from_bit_size utility method

[Alejandro Piñeiro]

Returns the brw_type for a given ssa.bit_size, and a reference type.
So if bit_size is 64, and the reference type is BRW_REGISTER_TYPE_F,
it returns BRW_REGISTER_TYPE_DF. The same applies if bit_size is 32
and reference type is BRW_REGISTER_TYPE_HF it returns BRW_REGISTER_TYPE_F

Signed-off-by: Jose Maria Casanova Crespo 
Signed-off-by: Alejandro Piñeiro num_array_elems == 0 ? 1 : reg->num_array_elems;
   unsigned size = array_elems * reg->num_components;
   const brw_reg_type reg_type =
- reg->bit_size == 32 ? BRW_REGISTER_TYPE_F : BRW_REGISTER_TYPE_DF;
+ brw_reg_type_from_bit_size(reg->bit_size, BRW_REGISTER_TYPE_F);
   nir_locals[reg->index] = bld.vgrf(reg_type, size);
}
 
@@ -1338,7 +1395,7 @@ fs_visitor::nir_emit_load_const(const fs_builder &bld,
 nir_load_const_instr *instr)
 {
const brw_reg_type reg_type =
-  instr->def.bit_size == 32 ? BRW_REGISTER_TYPE_D : BRW_REGISTER_TYPE_DF;
+  brw_reg_type_from_bit_size(instr->def.bit_size, BRW_REGISTER_TYPE_D);
fs_reg reg = bld.vgrf(reg_type, instr->def.num_components);
 
switch (instr->def.bit_size) {
@@ -1366,8 +1423,8 @@ fs_visitor::get_nir_src(const nir_src &src)
fs_reg reg;
if (src.is_ssa) {
   if (src.ssa->parent_instr->type == nir_instr_type_ssa_undef) {
- const brw_reg_type reg_type = src.ssa->bit_size == 32 ?
-BRW_REGISTER_TYPE_D : BRW_REGISTER_TYPE_DF;
+ const brw_reg_type reg_type =
+brw_reg_type_from_bit_size(src.ssa->bit_size, BRW_REGISTER_TYPE_D);
  reg = bld.vgrf(reg_type, src.ssa->num_components);
   } else {
  reg = nir_ssa_values[src.ssa->index];
@@ -1401,7 +1458,7 @@ fs_visitor::get_nir_dest(const nir_dest &dest)
 {
if (dest.is_ssa) {
   const brw_reg_type reg_type =
- dest.ssa.bit_size == 32 ? BRW_REGISTER_TYPE_F : BRW_REGISTER_TYPE_DF;
+ brw_reg_type_from_bit_size(dest.ssa.bit_size, BRW_REGISTER_TYPE_F);
   nir_ssa_values[dest.ssa.index] =
  bld.vgrf(reg_type, dest.ssa.num_components);
   return nir_ssa_values[dest.ssa.index];
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev