[Mesa-dev] [PATCH 10/59] intel/compiler: implement conversions from 16-bit float to 64-bit

[Iago Toral]

On Tue, 2018-12-04 at 18:10 +0200, Pohjolainen, Topi wrote:
> On Tue, Dec 04, 2018 at 02:33:25PM +0200, Pohjolainen, Topi wrote:
> > On Tue, Dec 04, 2018 at 08:16:34AM +0100, Iago Toral Quiroga wrote:
> > > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > > ---
> > >  src/intel/compiler/brw_fs_nir.cpp | 41
> > > +++++++++++++++++++++++++++++++
> > >  1 file changed, 41 insertions(+)
> > > 
> > > diff --git a/src/intel/compiler/brw_fs_nir.cpp
> > > b/src/intel/compiler/brw_fs_nir.cpp
> > > index 6eb68794f58..7294f49ddc0 100644
> > > --- a/src/intel/compiler/brw_fs_nir.cpp
> > > +++ b/src/intel/compiler/brw_fs_nir.cpp
> > > @@ -796,6 +796,47 @@ fs_visitor::nir_emit_alu(const fs_builder
> > > &bld, nir_alu_instr *instr)
> > >     case nir_op_f2f64:
> > >     case nir_op_f2i64:
> > >     case nir_op_f2u64:
> > > +      /* BDW PRM, vol02, Command Reference Instructions, mov -
> > > MOVE:
> > > +       *
> > > +       *   "There is no direct conversion from HF to DF or DF to
> > > HF.
> > > +       *    Use two instructions and F (Float) as an
> > > intermediate type.
> > > +       *
> > > +       *    There is no direct conversion from HF to Q/UQ or
> > > Q/UQ to HF.
> > > +       *    Use two instructions and F (Float) or a word integer
> > > type
> > > +       *    or a DWord integer type as an intermediate type."
> > > +       */
> > > +      if (nir_src_bit_size(instr->src[0].src) == 16) {
> > > +         fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_F, 1);
> > > +         inst = bld.MOV(tmp, op[0]);
> > > +         inst->saturate = instr->dest.saturate;
> > > +         op[0] = tmp;
> > > +      }
> > > +
> > > +      /* CHV PRM, vol07, 3D Media GPGPU Engine, Register Region
> > > Restrictions:
> > > +       *
> > > +       *    "When source or destination is 64b (...), regioning
> > > in Align1
> > > +       *     must follow these rules:
> > > +       *
> > > +       *     1. Source and destination horizontal stride must be
> > > aligned to
> > > +       *        the same qword.
> > > +       *     (...)"
> > > +       *
> > > +       * This means that conversions from bit-sizes smaller than
> > > 64-bit to
> > > +       * 64-bit need to have the source data elements aligned to
> > > 64-bit.
> > > +       * This restriction does not apply to BDW and later.
> > > +       */
> > > +      if (type_sz(result.type) == 8 && type_sz(op[0].type) < 8
> > > &&
> > > +          (devinfo->is_cherryview ||
> > > gen_device_info_is_9lp(devinfo))) {
> > > +         fs_reg tmp = bld.vgrf(result.type, 1);
> > > +         tmp = subscript(tmp, op[0].type, 0);
> > > +         inst = bld.MOV(tmp, op[0]);
> > > +         op[0] = tmp;
> > > +      }
> > 
> > For this second part we seem to have similar logic further down
> > after
> > "nir_op_u2u64" (not visible here) in master? Would it be possible
> > to fallthru
> > from here and re-use that?
> 
> And after reading it more carefully myself it looks that this is
> actually
> cleaner.
> 
> I noticed that in the nir_op_u2u64 case the destination and source
> sizes are
> checked using:
> 
>            if (nir_dest_bit_size(instr->dest.dest) == 64 &&
>                nir_src_bit_size(instr->src[0].src) < 64 &&
>                ...
> 
> Should we use the same here for consistency?

Right above this we can rewrite op[0] with a temporary that would be
different from instr->src[0].src so we can't check the nir sources any
more. Also, by the end of the series, when we incorporate 8-bit
conversions there will be more cases like this that we need to account
for and we end up rewriting the u2u64 case to this style as well.

Iago

> > 
> > > +
> > > +      inst = bld.MOV(result, op[0]);
> > > +      inst->saturate = instr->dest.saturate;
> > > +      break;
> > > +
> > >     case nir_op_i2f64:
> > >     case nir_op_i2i64:
> > >     case nir_op_u2f64:
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>