[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 <jmcasanova at igalia.com>
wrote:

> On 30/10/17 23:40, Jason Ekstrand wrote:
> > On Thu, Oct 12, 2017 at 11:38 AM, Jose Maria Casanova Crespo
> > <jmcasanova at igalia.com <mailto:jmcasanova at igalia.com>> wrote:
> >
> >     From: Alejandro Piñeiro <apinheiro at igalia.com
> >     <mailto:apinheiro at igalia.com>>
> >
> >     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 <jmcasanova at igalia.com
> >     <mailto:jmcasanova at igalia.com>>
> >     Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com
> >     <mailto:apinheiro at igalia.com>>
> >     ---
> >      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 at lists.freedesktop.org <mailto:mesa-dev at lists.
> freedesktop.org>
> >     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> >
> >
> >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171113/32be1ae3/attachment.html>