[Mesa-dev] [PATCH v2 13/53] intel/compiler: add a helper to handle conversions to 64-bit in atom

[Iago Toral]

Thanks Curro, I'll check it out.

Iago

On Wed, 2019-01-02 at 15:00 -0800, Francisco Jerez wrote:
> This patch is redundant with the regioning lowering pass I sent a few
> days ago [1].  The problem with this approach is that on the one hand
> it's easy for the back-end compiler to cause code which was legalized
> at
> NIR translation time to become illegal again accidentally, on the
> other
> hand there's the maintainability issue of having workarounds for the
> exact same restriction scattered all over the place.
> 
> Please drop it, there shouldn't be any need to do manual workarounds
> at
> NIR translation time for the CHV/BXT regioning restrictions to be
> honored anymore.
> 
> [1] 
> https://lists.freedesktop.org/archives/mesa-dev/2018-December/212775.html
> 
> Iago Toral Quiroga <itoral at igalia.com> writes:
> 
> > ---
> >  src/intel/compiler/brw_fs_nir.cpp | 55 ++++++++++++++++++---------
> > ----
> >  1 file changed, 33 insertions(+), 22 deletions(-)
> > 
> > diff --git a/src/intel/compiler/brw_fs_nir.cpp
> > b/src/intel/compiler/brw_fs_nir.cpp
> > index 92ec85a27cc..15715651aa6 100644
> > --- a/src/intel/compiler/brw_fs_nir.cpp
> > +++ b/src/intel/compiler/brw_fs_nir.cpp
> > @@ -664,6 +664,38 @@ brw_rnd_mode_from_nir_op (const nir_op op) {
> >     }
> >  }
> >  
> > +static bool
> > +fixup_64bit_conversion(const fs_builder &bld,
> > +                       fs_reg dst, fs_reg src,
> > +                       bool saturate,
> > +                       const struct gen_device_info *devinfo)
> > +{
> > +   /* 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(dst.type) == 8 && type_sz(src.type) < 8 &&
> > +       (devinfo->is_cherryview ||
> > gen_device_info_is_9lp(devinfo))) {
> > +      fs_reg tmp = bld.vgrf(dst.type, 1);
> > +      tmp = subscript(tmp, src.type, 0);
> > +      bld.MOV(tmp, src);
> > +      fs_inst *inst = bld.MOV(dst, tmp);
> > +      inst->saturate = saturate;
> > +      return true;
> > +   }
> > +
> > +   return false;
> > +}
> > +
> >  void
> >  fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr
> > *instr)
> >  {
> > @@ -805,29 +837,8 @@ fs_visitor::nir_emit_alu(const fs_builder
> > &bld, nir_alu_instr *instr)
> >     case nir_op_i2i64:
> >     case nir_op_u2f64:
> >     case nir_op_u2u64:
> > -      /* 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 (nir_dest_bit_size(instr->dest.dest) == 64 &&
> > -          nir_src_bit_size(instr->src[0].src) < 64 &&
> > -          (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]);
> > -         inst = bld.MOV(result, tmp);
> > -         inst->saturate = instr->dest.saturate;
> > +      if (fixup_64bit_conversion(bld, result, op[0], instr-
> > >dest.saturate, devinfo))
> >           break;
> > -      }
> >        /* fallthrough */
> >     case nir_op_f2f32:
> >     case nir_op_f2i32:
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev