[Mesa-dev] [PATCH v3 17/43] i965/fs: Enable rounding mode on f2f16 ops

[Jose Maria Casanova Crespo]

From: Alejandro Piñeiro 

By default we don't set the rounding mode. We only set
round-to-near-even or round-to-zero mode if explicitly set from nir.

v2: Use a single SHADER_OPCODE_RND_MODE opcode taking an immediate
with the rounding mode (Curro)

Signed-off-by: Jose Maria Casanova Crespo 
Signed-off-by: Alejandro Piñeiro 
---
 src/intel/compiler/brw_fs_nir.cpp | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/intel/compiler/brw_fs_nir.cpp 
b/src/intel/compiler/brw_fs_nir.cpp
index 6908c7ea02..b356836e80 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -693,6 +693,14 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, 
nir_alu_instr *instr)
   inst->saturate = instr->dest.saturate;
   break;
 
+   case nir_op_f2f16_rtne:
+   case nir_op_f2f16_rtz:
+  if (instr->op == nir_op_f2f16_rtz)
+ bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(), 
brw_imm_d(BRW_RND_MODE_RTZ));
+  else if (instr->op == nir_op_f2f16_rtne)
+ bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(), 
brw_imm_d(BRW_RND_MODE_RTNE));
+  /* fallthrough */
+
   /* In theory, it would be better to use BRW_OPCODE_F32TO16. Depending
* on the HW gen, it is a special hw opcode or just a MOV, and
* brw_F32TO16 (at brw_eu_emit) would do the work to chose.
-- 
2.13.6

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

Re: [Mesa-dev] [PATCH v3 17/43] i965/fs: Enable rounding mode on f2f16 ops

[Chema Casanova]

On 30/10/17 23:40, Jason Ekstrand wrote:
> On Thu, Oct 12, 2017 at 11:38 AM, Jose Maria Casanova Crespo
> mailto:jmcasan...@igalia.com>> wrote:
> 
> From: Alejandro Piñeiro  >
> 
> By default we don't set the rounding mode. We only set
> round-to-near-even or round-to-zero mode if explicitly set from nir.
> 
> v2: Use a single SHADER_OPCODE_RND_MODE opcode taking an immediate
>     with the rounding mode (Curro)
> 
> Signed-off-by: Jose Maria Casanova Crespo  >
> Signed-off-by: Alejandro Piñeiro  >
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 6908c7ea02..b356836e80 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -693,6 +693,14 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> nir_alu_instr *instr)
>        inst->saturate = instr->dest.saturate;
>        break;
> 
> +   case nir_op_f2f16_rtne:
> +   case nir_op_f2f16_rtz:
> +      if (instr->op == nir_op_f2f16_rtz)
> +         bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> brw_imm_d(BRW_RND_MODE_RTZ));
> +      else if (instr->op == nir_op_f2f16_rtne)
> +         bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> brw_imm_d(BRW_RND_MODE_RTNE));
> +      /* fallthrough */
> 
> 
> It might look a little nicer (though it's more lines of code) to have a
> little brw_from_nir_rounding_mode helper and then we could have just the
> one emit call.  I don't care too much though.


What about this helper?

static brw_rnd_mode
brw_rnd_mode_from_nir_op (const nir_op op) {
   switch (op) {
   case nir_op_f2f16_rtz:
  return BRW_RND_MODE_RTZ;
   case nir_op_f2f16_rtne:
  return BRW_RND_MODE_RTNE;
   default:
  unreachable("Operation doesn't support rounding mode");
   }
}

And ...

   case nir_op_f2f16_rtne:
   case nir_op_f2f16_rtz:
  bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
   brw_imm_d(brw_rnd_mode_from_nir_op(instr->op)));


> 
> +
>        /* In theory, it would be better to use BRW_OPCODE_F32TO16.
> Depending
>         * on the HW gen, it is a special hw opcode or just a MOV, and
>         * brw_F32TO16 (at brw_eu_emit) would do the work to chose.
> --
> 2.13.6
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
> 
> 
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 17/43] i965/fs: Enable rounding mode on f2f16 ops

[Jason Ekstrand]

On Mon, Nov 13, 2017 at 12:41 PM, Chema Casanova 
wrote:

> On 30/10/17 23:40, Jason Ekstrand wrote:
> > On Thu, Oct 12, 2017 at 11:38 AM, Jose Maria Casanova Crespo
> > mailto:jmcasan...@igalia.com>> wrote:
> >
> > From: Alejandro Piñeiro  > >
> >
> > By default we don't set the rounding mode. We only set
> > round-to-near-even or round-to-zero mode if explicitly set from nir.
> >
> > v2: Use a single SHADER_OPCODE_RND_MODE opcode taking an immediate
> > with the rounding mode (Curro)
> >
> > Signed-off-by: Jose Maria Casanova Crespo  > >
> > Signed-off-by: Alejandro Piñeiro  > >
> > ---
> >  src/intel/compiler/brw_fs_nir.cpp | 8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/src/intel/compiler/brw_fs_nir.cpp
> > b/src/intel/compiler/brw_fs_nir.cpp
> > index 6908c7ea02..b356836e80 100644
> > --- a/src/intel/compiler/brw_fs_nir.cpp
> > +++ b/src/intel/compiler/brw_fs_nir.cpp
> > @@ -693,6 +693,14 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> > nir_alu_instr *instr)
> >inst->saturate = instr->dest.saturate;
> >break;
> >
> > +   case nir_op_f2f16_rtne:
> > +   case nir_op_f2f16_rtz:
> > +  if (instr->op == nir_op_f2f16_rtz)
> > + bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> > brw_imm_d(BRW_RND_MODE_RTZ));
> > +  else if (instr->op == nir_op_f2f16_rtne)
> > + bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> > brw_imm_d(BRW_RND_MODE_RTNE));
> > +  /* fallthrough */
> >
> >
> > It might look a little nicer (though it's more lines of code) to have a
> > little brw_from_nir_rounding_mode helper and then we could have just the
> > one emit call.  I don't care too much though.
>
>
> What about this helper?
>
> static brw_rnd_mode
> brw_rnd_mode_from_nir_op (const nir_op op) {
>switch (op) {
>case nir_op_f2f16_rtz:
>   return BRW_RND_MODE_RTZ;
>case nir_op_f2f16_rtne:
>   return BRW_RND_MODE_RTNE;
>default:
>   unreachable("Operation doesn't support rounding mode");
>}
> }
>
> And ...
>
>case nir_op_f2f16_rtne:
>case nir_op_f2f16_rtz:
>   bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
>brw_imm_d(brw_rnd_mode_from_nir_op(instr->op)));
>

Sounds good to me.


> >
> > +
> >/* In theory, it would be better to use BRW_OPCODE_F32TO16.
> > Depending
> > * on the HW gen, it is a special hw opcode or just a MOV, and
> > * brw_F32TO16 (at brw_eu_emit) would do the work to chose.
> > --
> > 2.13.6
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org  freedesktop.org>
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> >
> >
> >
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 17/43] i965/fs: Enable rounding mode on f2f16 ops

[Jason Ekstrand]

On Thu, Oct 12, 2017 at 11:38 AM, Jose Maria Casanova Crespo <
jmcasan...@igalia.com> wrote:

> From: Alejandro Piñeiro 
>
> By default we don't set the rounding mode. We only set
> round-to-near-even or round-to-zero mode if explicitly set from nir.
>
> v2: Use a single SHADER_OPCODE_RND_MODE opcode taking an immediate
> with the rounding mode (Curro)
>
> Signed-off-by: Jose Maria Casanova Crespo 
> Signed-off-by: Alejandro Piñeiro 
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 6908c7ea02..b356836e80 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -693,6 +693,14 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> nir_alu_instr *instr)
>inst->saturate = instr->dest.saturate;
>break;
>
> +   case nir_op_f2f16_rtne:
> +   case nir_op_f2f16_rtz:
> +  if (instr->op == nir_op_f2f16_rtz)
> + bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> brw_imm_d(BRW_RND_MODE_RTZ));
> +  else if (instr->op == nir_op_f2f16_rtne)
> + bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> brw_imm_d(BRW_RND_MODE_RTNE));
> +  /* fallthrough */
>

It might look a little nicer (though it's more lines of code) to have a
little brw_from_nir_rounding_mode helper and then we could have just the
one emit call.  I don't care too much though.


> +
>/* In theory, it would be better to use BRW_OPCODE_F32TO16.
> Depending
> * on the HW gen, it is a special hw opcode or just a MOV, and
> * brw_F32TO16 (at brw_eu_emit) would do the work to chose.
> --
> 2.13.6
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 17/43] i965/fs: Enable rounding mode on f2f16 ops

[Chema Casanova]

El 30/10/17 a las 23:40, Jason Ekstrand escribió:
> On Thu, Oct 12, 2017 at 11:38 AM, Jose Maria Casanova Crespo
> mailto:jmcasan...@igalia.com>> wrote:
>
> From: Alejandro Piñeiro  >
>
> By default we don't set the rounding mode. We only set
> round-to-near-even or round-to-zero mode if explicitly set from nir.
>
> v2: Use a single SHADER_OPCODE_RND_MODE opcode taking an immediate
>     with the rounding mode (Curro)
>
> Signed-off-by: Jose Maria Casanova Crespo  >
> Signed-off-by: Alejandro Piñeiro  >
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 6908c7ea02..b356836e80 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -693,6 +693,14 @@ fs_visitor::nir_emit_alu(const fs_builder
> &bld, nir_alu_instr *instr)
>        inst->saturate = instr->dest.saturate;
>        break;
>
> +   case nir_op_f2f16_rtne:
> +   case nir_op_f2f16_rtz:
> +      if (instr->op == nir_op_f2f16_rtz)
> +         bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> brw_imm_d(BRW_RND_MODE_RTZ));
> +      else if (instr->op == nir_op_f2f16_rtne)
> +         bld.emit(SHADER_OPCODE_RND_MODE, bld.null_reg_ud(),
> brw_imm_d(BRW_RND_MODE_RTNE));
> +      /* fallthrough */
>
>
> It might look a little nicer (though it's more lines of code) to have
> a little brw_from_nir_rounding_mode helper and then we could have just
> the one emit call.  I don't care too much though.

I agree, and it could simplify if we want to enable other rounding modes
supported by the hardware in the future.

>  
>
> +
>        /* In theory, it would be better to use BRW_OPCODE_F32TO16.
> Depending
>         * on the HW gen, it is a special hw opcode or just a MOV, and
>         * brw_F32TO16 (at brw_eu_emit) would do the work to chose.
> --
> 2.13.6
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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