[Mesa-dev] [PATCH v4 33/40] intel/compiler: also set F execution type for mixed float mode in BDW

[Iago Toral]

On Tue, 2019-02-26 at 13:55 -0800, Francisco Jerez wrote:
> Iago Toral Quiroga <itoral at igalia.com> writes:
> 
> > The section 'Execution Data Types' of 3D Media GPGPU volume, which
> > describes execution types, is exactly the same in BDW and SKL+.
> > 
> > Also, this section states that there is a single execution type, so
> > it
> > makes sense that this is the wider of the two floating point types
> > involved in mixed float mode, which is what we do for SKL+ and CHV.
> > ---
> >  src/intel/compiler/brw_eu_validate.c | 18 +++++++-----------
> >  1 file changed, 7 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/intel/compiler/brw_eu_validate.c
> > b/src/intel/compiler/brw_eu_validate.c
> > index 358a0347a93..000a05cb6ac 100644
> > --- a/src/intel/compiler/brw_eu_validate.c
> > +++ b/src/intel/compiler/brw_eu_validate.c
> > @@ -431,18 +431,14 @@ execution_type(const struct gen_device_info
> > *devinfo, const brw_inst *inst)
> >         src1_exec_type == BRW_REGISTER_TYPE_DF)
> >        return BRW_REGISTER_TYPE_DF;
> >  
> > -   if (devinfo->gen >= 9 || devinfo->is_cherryview) {
> > -      if (dst_exec_type == BRW_REGISTER_TYPE_F ||
> > -          src0_exec_type == BRW_REGISTER_TYPE_F ||
> > -          src1_exec_type == BRW_REGISTER_TYPE_F) {
> > -         return BRW_REGISTER_TYPE_F;
> > -      } else {
> > -         return BRW_REGISTER_TYPE_HF;
> > -      }
> > +   if (dst_exec_type == BRW_REGISTER_TYPE_F ||
> > +       src0_exec_type == BRW_REGISTER_TYPE_F ||
> > +       src1_exec_type == BRW_REGISTER_TYPE_F) {
> > +      return BRW_REGISTER_TYPE_F;
> > +   } else {
> > +      assert(devinfo->gen >= 8 && src0_exec_type ==
> > BRW_REGISTER_TYPE_HF);
> > +      return BRW_REGISTER_TYPE_HF;
> 
> I'm having trouble convincing myself that this is correct.  Aren't
> there
> four earlier return statements in this function you may potentially
> hit
> on BDW that will still fail to consider the destination type for
> instructions with HF operands?

Yes, and it seems this is not only in BDW actually, at least for 2-
source instructions.  I'll fix that.

BTW, I also see we have this code (right above this hunk):

/* Execution data type is independent of destination data type, 
 * except in mixed F/HF instructions on CHV and SKL+.
(...)
if ((devinfo->gen >= 9 || devinfo->is_cherryview) &&
     src0_exec_type == BRW_REGISTER_TYPE_HF) {
   return dst_exec_type;
}

Which suggests that we are considering a conversion from HF to F a
mixed float instruction, is that correct? Until now I have been
assuming that conversion MOVs like this were not mixed mode
instructions, just regular conversions between types.

Iago

> >     }
> > -
> > -   assert(src0_exec_type == BRW_REGISTER_TYPE_F);
> > -   return BRW_REGISTER_TYPE_F;
> >  }
> >  
> >  /**
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev