Re: [Mesa-dev] [PATCH v6 32/35] intel/compiler: validate region restrictions for mixed float mode

2019-04-13 Thread Francisco Jerez
"Juan A. Suarez Romero"  writes:

> On Wed, 2019-04-10 at 17:13 -0700, Francisco Jerez wrote:
>> "Juan A. Suarez Romero"  writes:
>> 
>> > From: Iago Toral Quiroga 
>> > 
>> > v2:
>> >  - Adapted unit tests to make them consistent with the changes done
>> >to the validation of half-float conversions.
>> > 
>> > v3 (Curro):
>> > - Check all the accummulators
>> > - Constify declarations
>> > - Do not check src1 type in single-source instructions.
>> > - Check for all instructions that read accumulator (either implicitly or
>> >   explicitly)
>> > - Check restrictions in src1 too.
>> > - Merge conditional block
>> > - Add invalid test case.
>> > ---
>> >  src/intel/compiler/brw_eu_validate.c| 290 +++
>> >  src/intel/compiler/test_eu_validate.cpp | 631 
>> >  2 files changed, 921 insertions(+)
>> > 
>> > diff --git a/src/intel/compiler/brw_eu_validate.c 
>> > b/src/intel/compiler/brw_eu_validate.c
>> > index cfaf126e2f5..4a735641c86 100644
>> > --- a/src/intel/compiler/brw_eu_validate.c
>> > +++ b/src/intel/compiler/brw_eu_validate.c
>> > @@ -170,6 +170,20 @@ src1_is_null(const struct gen_device_info *devinfo, 
>> > const brw_inst *inst)
>> >brw_inst_src1_da_reg_nr(devinfo, inst) == BRW_ARF_NULL;
>> >  }
>> >  
>> > +static bool
>> > +src0_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
>> > +{
>> > +   return brw_inst_src0_reg_file(devinfo, inst) == 
>> > BRW_ARCHITECTURE_REGISTER_FILE &&
>> > +  (brw_inst_src0_da_reg_nr(devinfo, inst) & 0xF0) == 
>> > BRW_ARF_ACCUMULATOR;
>> > +}
>> > +
>> > +static bool
>> > +src1_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
>> > +{
>> > +   return brw_inst_src1_reg_file(devinfo, inst) == 
>> > BRW_ARCHITECTURE_REGISTER_FILE &&
>> > +  (brw_inst_src1_da_reg_nr(devinfo, inst) & 0xF0) == 
>> > BRW_ARF_ACCUMULATOR;
>> > +}
>> > +
>> >  static bool
>> >  src0_is_grf(const struct gen_device_info *devinfo, const brw_inst *inst)
>> >  {
>> > @@ -275,6 +289,27 @@ sources_not_null(const struct gen_device_info 
>> > *devinfo,
>> > return error_msg;
>> >  }
>> >  
>> > +static bool
>> > +inst_uses_src_acc(const struct gen_device_info *devinfo, const brw_inst 
>> > *inst)
>> > +{
>> > +   /* Check instructions that use implicit accumulator sources */
>> > +   switch (brw_inst_opcode(devinfo, inst)) {
>> > +   case BRW_OPCODE_MAC:
>> > +   case BRW_OPCODE_MACH:
>> > +   case BRW_OPCODE_SADA2:
>> > +  return true;
>> > +   }
>> > +
>> > +   /* Instructions with three source operands cannot use explicit 
>> > accumulator
>> > +* operands.
>> > +*/
>> 
>> They can on Gen10+.  Yeah, I know, it's quite a pain to have to
>> special-case 3src instructions everywhere in the validator code...
>
>
> Checking other parts of code, I'll assume that srcN_is_acc() should return 
> false
> for align16 mode; at least in them there're assertions that in this mode srcs
> can only be GRF.
>

Yes, only Align1 3-source instructions on Gen10 and greater can use the
accumulator explicitly.

> OTOH, is it worth to handle here the case for 3src instructions allowing
> explicit accumulator? If other parts of drive asume this is not possible, I
> understand it would be better to fix this in all the code in a separate 
> patchset
> (not related with float16).


I think it's at least worth adding an assert(num_sources < 3) and a
FINISHME comment so whenever this function starts getting used to
validate 3-source instructions people notice it's incomplete.

Honestly it's kind of disturbing that 3-source instructions aren't being
validated at all for the most part in the EU validator.  But that's not
really your fault.

[A bunch more comments below]

>
>
>> 
>> > +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
>> > +   if (num_sources > 2)
>> > +  return false;
>> > +
>> > +   return src0_is_acc(devinfo, inst) || (num_sources > 1 && 
>> > src1_is_acc(devinfo, inst));
>> > +}
>> > +
>> >  static struct string
>> >  send_restrictions(const struct gen_device_info *devinfo,
>> >const brw_inst *inst)
>> > @@ -938,6 +973,260 @@ general_restrictions_on_region_parameters(const 
>> > struct gen_device_info *devinfo,
>> > return error_msg;
>> >  }
>> >  
>> > +static struct string
>> > +special_restrictions_for_mixed_float_mode(const struct gen_device_info 
>> > *devinfo,
>> > +  const brw_inst *inst)
>> > +{
>> > +   struct string error_msg = { .str = NULL, .len = 0 };
>> > +
>> > +   const unsigned opcode = brw_inst_opcode(devinfo, inst);
>> > +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
>> > +   if (num_sources >= 3)
>> > +  return error_msg;
>> > +
>> > +   if (!is_mixed_float(devinfo, inst))
>> > +  return error_msg;
>> > +
>> > +   unsigned exec_size = 1 << brw_inst_exec_size(devinfo, inst);
>> > +   bool is_align16 = brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_16;
>> 

Re: [Mesa-dev] [PATCH v6 32/35] intel/compiler: validate region restrictions for mixed float mode

2019-04-11 Thread Juan A. Suarez Romero
On Wed, 2019-04-10 at 17:13 -0700, Francisco Jerez wrote:
> "Juan A. Suarez Romero"  writes:
> 
> > From: Iago Toral Quiroga 
> > 
> > v2:
> >  - Adapted unit tests to make them consistent with the changes done
> >to the validation of half-float conversions.
> > 
> > v3 (Curro):
> > - Check all the accummulators
> > - Constify declarations
> > - Do not check src1 type in single-source instructions.
> > - Check for all instructions that read accumulator (either implicitly or
> >   explicitly)
> > - Check restrictions in src1 too.
> > - Merge conditional block
> > - Add invalid test case.
> > ---
> >  src/intel/compiler/brw_eu_validate.c| 290 +++
> >  src/intel/compiler/test_eu_validate.cpp | 631 
> >  2 files changed, 921 insertions(+)
> > 
> > diff --git a/src/intel/compiler/brw_eu_validate.c 
> > b/src/intel/compiler/brw_eu_validate.c
> > index cfaf126e2f5..4a735641c86 100644
> > --- a/src/intel/compiler/brw_eu_validate.c
> > +++ b/src/intel/compiler/brw_eu_validate.c
> > @@ -170,6 +170,20 @@ src1_is_null(const struct gen_device_info *devinfo, 
> > const brw_inst *inst)
> >brw_inst_src1_da_reg_nr(devinfo, inst) == BRW_ARF_NULL;
> >  }
> >  
> > +static bool
> > +src0_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
> > +{
> > +   return brw_inst_src0_reg_file(devinfo, inst) == 
> > BRW_ARCHITECTURE_REGISTER_FILE &&
> > +  (brw_inst_src0_da_reg_nr(devinfo, inst) & 0xF0) == 
> > BRW_ARF_ACCUMULATOR;
> > +}
> > +
> > +static bool
> > +src1_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
> > +{
> > +   return brw_inst_src1_reg_file(devinfo, inst) == 
> > BRW_ARCHITECTURE_REGISTER_FILE &&
> > +  (brw_inst_src1_da_reg_nr(devinfo, inst) & 0xF0) == 
> > BRW_ARF_ACCUMULATOR;
> > +}
> > +
> >  static bool
> >  src0_is_grf(const struct gen_device_info *devinfo, const brw_inst *inst)
> >  {
> > @@ -275,6 +289,27 @@ sources_not_null(const struct gen_device_info *devinfo,
> > return error_msg;
> >  }
> >  
> > +static bool
> > +inst_uses_src_acc(const struct gen_device_info *devinfo, const brw_inst 
> > *inst)
> > +{
> > +   /* Check instructions that use implicit accumulator sources */
> > +   switch (brw_inst_opcode(devinfo, inst)) {
> > +   case BRW_OPCODE_MAC:
> > +   case BRW_OPCODE_MACH:
> > +   case BRW_OPCODE_SADA2:
> > +  return true;
> > +   }
> > +
> > +   /* Instructions with three source operands cannot use explicit 
> > accumulator
> > +* operands.
> > +*/
> 
> They can on Gen10+.  Yeah, I know, it's quite a pain to have to
> special-case 3src instructions everywhere in the validator code...


Checking other parts of code, I'll assume that srcN_is_acc() should return false
for align16 mode; at least in them there're assertions that in this mode srcs
can only be GRF.

OTOH, is it worth to handle here the case for 3src instructions allowing
explicit accumulator? If other parts of drive asume this is not possible, I
understand it would be better to fix this in all the code in a separate patchset
(not related with float16).


> 
> > +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
> > +   if (num_sources > 2)
> > +  return false;
> > +
> > +   return src0_is_acc(devinfo, inst) || (num_sources > 1 && 
> > src1_is_acc(devinfo, inst));
> > +}
> > +
> >  static struct string
> >  send_restrictions(const struct gen_device_info *devinfo,
> >const brw_inst *inst)
> > @@ -938,6 +973,260 @@ general_restrictions_on_region_parameters(const 
> > struct gen_device_info *devinfo,
> > return error_msg;
> >  }
> >  
> > +static struct string
> > +special_restrictions_for_mixed_float_mode(const struct gen_device_info 
> > *devinfo,
> > +  const brw_inst *inst)
> > +{
> > +   struct string error_msg = { .str = NULL, .len = 0 };
> > +
> > +   const unsigned opcode = brw_inst_opcode(devinfo, inst);
> > +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
> > +   if (num_sources >= 3)
> > +  return error_msg;
> > +
> > +   if (!is_mixed_float(devinfo, inst))
> > +  return error_msg;
> > +
> > +   unsigned exec_size = 1 << brw_inst_exec_size(devinfo, inst);
> > +   bool is_align16 = brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_16;
> > +
> > +   enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst);
> > +   enum brw_reg_type src1_type = num_sources > 1 ?
> > + brw_inst_src1_type(devinfo, inst) : 0;
> > +   enum brw_reg_type dst_type = brw_inst_dst_type(devinfo, inst);
> > +
> > +   unsigned dst_stride = STRIDE(brw_inst_dst_hstride(devinfo, inst));
> > +   bool dst_is_packed = is_packed(exec_size * dst_stride, exec_size, 
> > dst_stride);
> > +
> > +   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
> > +* Float Operations:
> > +*
> > +*"Indirect addressing on source is not supported when source and
> > +* 

Re: [Mesa-dev] [PATCH v6 32/35] intel/compiler: validate region restrictions for mixed float mode

2019-04-11 Thread Juan A. Suarez Romero
On Wed, 2019-04-10 at 17:13 -0700, Francisco Jerez wrote:
> "Juan A. Suarez Romero"  writes:
> 
> > From: Iago Toral Quiroga 
> > 
> > v2:
> >  - Adapted unit tests to make them consistent with the changes done
> >to the validation of half-float conversions.
> > 
> > v3 (Curro):
> > - Check all the accummulators
> > - Constify declarations
> > - Do not check src1 type in single-source instructions.
> > - Check for all instructions that read accumulator (either implicitly or
> >   explicitly)
> > - Check restrictions in src1 too.
> > - Merge conditional block
> > - Add invalid test case.
> > ---
> >  src/intel/compiler/brw_eu_validate.c| 290 +++
> >  src/intel/compiler/test_eu_validate.cpp | 631 
> >  2 files changed, 921 insertions(+)
> > 
> > diff --git a/src/intel/compiler/brw_eu_validate.c 
> > b/src/intel/compiler/brw_eu_validate.c
> > index cfaf126e2f5..4a735641c86 100644
> > --- a/src/intel/compiler/brw_eu_validate.c
> > +++ b/src/intel/compiler/brw_eu_validate.c
> > @@ -170,6 +170,20 @@ src1_is_null(const struct gen_device_info *devinfo, 
> > const brw_inst *inst)
> >brw_inst_src1_da_reg_nr(devinfo, inst) == BRW_ARF_NULL;
> >  }
> >  
> > +static bool
> > +src0_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
> > +{
> > +   return brw_inst_src0_reg_file(devinfo, inst) == 
> > BRW_ARCHITECTURE_REGISTER_FILE &&
> > +  (brw_inst_src0_da_reg_nr(devinfo, inst) & 0xF0) == 
> > BRW_ARF_ACCUMULATOR;
> > +}
> > +
> > +static bool
> > +src1_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
> > +{
> > +   return brw_inst_src1_reg_file(devinfo, inst) == 
> > BRW_ARCHITECTURE_REGISTER_FILE &&
> > +  (brw_inst_src1_da_reg_nr(devinfo, inst) & 0xF0) == 
> > BRW_ARF_ACCUMULATOR;
> > +}
> > +
> >  static bool
> >  src0_is_grf(const struct gen_device_info *devinfo, const brw_inst *inst)
> >  {
> > @@ -275,6 +289,27 @@ sources_not_null(const struct gen_device_info *devinfo,
> > return error_msg;
> >  }
> >  
> > +static bool
> > +inst_uses_src_acc(const struct gen_device_info *devinfo, const brw_inst 
> > *inst)
> > +{
> > +   /* Check instructions that use implicit accumulator sources */
> > +   switch (brw_inst_opcode(devinfo, inst)) {
> > +   case BRW_OPCODE_MAC:
> > +   case BRW_OPCODE_MACH:
> > +   case BRW_OPCODE_SADA2:
> > +  return true;
> > +   }
> > +
> > +   /* Instructions with three source operands cannot use explicit 
> > accumulator
> > +* operands.
> > +*/
> 
> They can on Gen10+.  Yeah, I know, it's quite a pain to have to
> special-case 3src instructions everywhere in the validator code...

Is this strictly for Gen>10 or includes Gen10? In the Gen10 PRM still says that
3-src opearand instructions cannot use explicit accumulator


> 
> > +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
> > +   if (num_sources > 2)
> > +  return false;
> > +
> > +   return src0_is_acc(devinfo, inst) || (num_sources > 1 && 
> > src1_is_acc(devinfo, inst));
> > +}
> > +
> >  static struct string
> >  send_restrictions(const struct gen_device_info *devinfo,
> >const brw_inst *inst)
> > @@ -938,6 +973,260 @@ general_restrictions_on_region_parameters(const 
> > struct gen_device_info *devinfo,
> > return error_msg;
> >  }
> >  
> > +static struct string
> > +special_restrictions_for_mixed_float_mode(const struct gen_device_info 
> > *devinfo,
> > +  const brw_inst *inst)
> > +{
> > +   struct string error_msg = { .str = NULL, .len = 0 };
> > +
> > +   const unsigned opcode = brw_inst_opcode(devinfo, inst);
> > +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
> > +   if (num_sources >= 3)
> > +  return error_msg;
> > +
> > +   if (!is_mixed_float(devinfo, inst))
> > +  return error_msg;
> > +
> > +   unsigned exec_size = 1 << brw_inst_exec_size(devinfo, inst);
> > +   bool is_align16 = brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_16;
> > +
> > +   enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst);
> > +   enum brw_reg_type src1_type = num_sources > 1 ?
> > + brw_inst_src1_type(devinfo, inst) : 0;
> > +   enum brw_reg_type dst_type = brw_inst_dst_type(devinfo, inst);
> > +
> > +   unsigned dst_stride = STRIDE(brw_inst_dst_hstride(devinfo, inst));
> > +   bool dst_is_packed = is_packed(exec_size * dst_stride, exec_size, 
> > dst_stride);
> > +
> > +   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
> > +* Float Operations:
> > +*
> > +*"Indirect addressing on source is not supported when source and
> > +* destination data types are mixed float."
> > +*/
> > +   ERROR_IF((types_are_mixed_float(dst_type, src0_type) &&
> > + brw_inst_src0_address_mode(devinfo, inst) != 
> > BRW_ADDRESS_DIRECT) ||
> > +(num_sources > 1 &&
> > + types_are_mixed_float(dst_type, 

Re: [Mesa-dev] [PATCH v6 32/35] intel/compiler: validate region restrictions for mixed float mode

2019-04-10 Thread Francisco Jerez
"Juan A. Suarez Romero"  writes:

> From: Iago Toral Quiroga 
>
> v2:
>  - Adapted unit tests to make them consistent with the changes done
>to the validation of half-float conversions.
>
> v3 (Curro):
> - Check all the accummulators
> - Constify declarations
> - Do not check src1 type in single-source instructions.
> - Check for all instructions that read accumulator (either implicitly or
>   explicitly)
> - Check restrictions in src1 too.
> - Merge conditional block
> - Add invalid test case.
> ---
>  src/intel/compiler/brw_eu_validate.c| 290 +++
>  src/intel/compiler/test_eu_validate.cpp | 631 
>  2 files changed, 921 insertions(+)
>
> diff --git a/src/intel/compiler/brw_eu_validate.c 
> b/src/intel/compiler/brw_eu_validate.c
> index cfaf126e2f5..4a735641c86 100644
> --- a/src/intel/compiler/brw_eu_validate.c
> +++ b/src/intel/compiler/brw_eu_validate.c
> @@ -170,6 +170,20 @@ src1_is_null(const struct gen_device_info *devinfo, 
> const brw_inst *inst)
>brw_inst_src1_da_reg_nr(devinfo, inst) == BRW_ARF_NULL;
>  }
>  
> +static bool
> +src0_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
> +{
> +   return brw_inst_src0_reg_file(devinfo, inst) == 
> BRW_ARCHITECTURE_REGISTER_FILE &&
> +  (brw_inst_src0_da_reg_nr(devinfo, inst) & 0xF0) == 
> BRW_ARF_ACCUMULATOR;
> +}
> +
> +static bool
> +src1_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
> +{
> +   return brw_inst_src1_reg_file(devinfo, inst) == 
> BRW_ARCHITECTURE_REGISTER_FILE &&
> +  (brw_inst_src1_da_reg_nr(devinfo, inst) & 0xF0) == 
> BRW_ARF_ACCUMULATOR;
> +}
> +
>  static bool
>  src0_is_grf(const struct gen_device_info *devinfo, const brw_inst *inst)
>  {
> @@ -275,6 +289,27 @@ sources_not_null(const struct gen_device_info *devinfo,
> return error_msg;
>  }
>  
> +static bool
> +inst_uses_src_acc(const struct gen_device_info *devinfo, const brw_inst 
> *inst)
> +{
> +   /* Check instructions that use implicit accumulator sources */
> +   switch (brw_inst_opcode(devinfo, inst)) {
> +   case BRW_OPCODE_MAC:
> +   case BRW_OPCODE_MACH:
> +   case BRW_OPCODE_SADA2:
> +  return true;
> +   }
> +
> +   /* Instructions with three source operands cannot use explicit accumulator
> +* operands.
> +*/

They can on Gen10+.  Yeah, I know, it's quite a pain to have to
special-case 3src instructions everywhere in the validator code...

> +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
> +   if (num_sources > 2)
> +  return false;
> +
> +   return src0_is_acc(devinfo, inst) || (num_sources > 1 && 
> src1_is_acc(devinfo, inst));
> +}
> +
>  static struct string
>  send_restrictions(const struct gen_device_info *devinfo,
>const brw_inst *inst)
> @@ -938,6 +973,260 @@ general_restrictions_on_region_parameters(const struct 
> gen_device_info *devinfo,
> return error_msg;
>  }
>  
> +static struct string
> +special_restrictions_for_mixed_float_mode(const struct gen_device_info 
> *devinfo,
> +  const brw_inst *inst)
> +{
> +   struct string error_msg = { .str = NULL, .len = 0 };
> +
> +   const unsigned opcode = brw_inst_opcode(devinfo, inst);
> +   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
> +   if (num_sources >= 3)
> +  return error_msg;
> +
> +   if (!is_mixed_float(devinfo, inst))
> +  return error_msg;
> +
> +   unsigned exec_size = 1 << brw_inst_exec_size(devinfo, inst);
> +   bool is_align16 = brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_16;
> +
> +   enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst);
> +   enum brw_reg_type src1_type = num_sources > 1 ?
> + brw_inst_src1_type(devinfo, inst) : 0;
> +   enum brw_reg_type dst_type = brw_inst_dst_type(devinfo, inst);
> +
> +   unsigned dst_stride = STRIDE(brw_inst_dst_hstride(devinfo, inst));
> +   bool dst_is_packed = is_packed(exec_size * dst_stride, exec_size, 
> dst_stride);
> +
> +   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
> +* Float Operations:
> +*
> +*"Indirect addressing on source is not supported when source and
> +* destination data types are mixed float."
> +*/
> +   ERROR_IF((types_are_mixed_float(dst_type, src0_type) &&
> + brw_inst_src0_address_mode(devinfo, inst) != 
> BRW_ADDRESS_DIRECT) ||
> +(num_sources > 1 &&
> + types_are_mixed_float(dst_type, src1_type) &&
> + brw_inst_src1_address_mode(devinfo, inst) != 
> BRW_ADDRESS_DIRECT),
> +"Indirect addressing on source is not supported when source and "
> +"destination data types are mixed float");
> +
> +   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
> +* Float Operations:
> +*
> +*"No SIMD16 in mixed mode when destination is f32. Instruction
> +* execution size must be no more than 

[Mesa-dev] [PATCH v6 32/35] intel/compiler: validate region restrictions for mixed float mode

2019-04-03 Thread Juan A. Suarez Romero
From: Iago Toral Quiroga 

v2:
 - Adapted unit tests to make them consistent with the changes done
   to the validation of half-float conversions.

v3 (Curro):
- Check all the accummulators
- Constify declarations
- Do not check src1 type in single-source instructions.
- Check for all instructions that read accumulator (either implicitly or
  explicitly)
- Check restrictions in src1 too.
- Merge conditional block
- Add invalid test case.
---
 src/intel/compiler/brw_eu_validate.c| 290 +++
 src/intel/compiler/test_eu_validate.cpp | 631 
 2 files changed, 921 insertions(+)

diff --git a/src/intel/compiler/brw_eu_validate.c 
b/src/intel/compiler/brw_eu_validate.c
index cfaf126e2f5..4a735641c86 100644
--- a/src/intel/compiler/brw_eu_validate.c
+++ b/src/intel/compiler/brw_eu_validate.c
@@ -170,6 +170,20 @@ src1_is_null(const struct gen_device_info *devinfo, const 
brw_inst *inst)
   brw_inst_src1_da_reg_nr(devinfo, inst) == BRW_ARF_NULL;
 }
 
+static bool
+src0_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   return brw_inst_src0_reg_file(devinfo, inst) == 
BRW_ARCHITECTURE_REGISTER_FILE &&
+  (brw_inst_src0_da_reg_nr(devinfo, inst) & 0xF0) == 
BRW_ARF_ACCUMULATOR;
+}
+
+static bool
+src1_is_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   return brw_inst_src1_reg_file(devinfo, inst) == 
BRW_ARCHITECTURE_REGISTER_FILE &&
+  (brw_inst_src1_da_reg_nr(devinfo, inst) & 0xF0) == 
BRW_ARF_ACCUMULATOR;
+}
+
 static bool
 src0_is_grf(const struct gen_device_info *devinfo, const brw_inst *inst)
 {
@@ -275,6 +289,27 @@ sources_not_null(const struct gen_device_info *devinfo,
return error_msg;
 }
 
+static bool
+inst_uses_src_acc(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   /* Check instructions that use implicit accumulator sources */
+   switch (brw_inst_opcode(devinfo, inst)) {
+   case BRW_OPCODE_MAC:
+   case BRW_OPCODE_MACH:
+   case BRW_OPCODE_SADA2:
+  return true;
+   }
+
+   /* Instructions with three source operands cannot use explicit accumulator
+* operands.
+*/
+   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
+   if (num_sources > 2)
+  return false;
+
+   return src0_is_acc(devinfo, inst) || (num_sources > 1 && 
src1_is_acc(devinfo, inst));
+}
+
 static struct string
 send_restrictions(const struct gen_device_info *devinfo,
   const brw_inst *inst)
@@ -938,6 +973,260 @@ general_restrictions_on_region_parameters(const struct 
gen_device_info *devinfo,
return error_msg;
 }
 
+static struct string
+special_restrictions_for_mixed_float_mode(const struct gen_device_info 
*devinfo,
+  const brw_inst *inst)
+{
+   struct string error_msg = { .str = NULL, .len = 0 };
+
+   const unsigned opcode = brw_inst_opcode(devinfo, inst);
+   const unsigned num_sources = num_sources_from_inst(devinfo, inst);
+   if (num_sources >= 3)
+  return error_msg;
+
+   if (!is_mixed_float(devinfo, inst))
+  return error_msg;
+
+   unsigned exec_size = 1 << brw_inst_exec_size(devinfo, inst);
+   bool is_align16 = brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_16;
+
+   enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst);
+   enum brw_reg_type src1_type = num_sources > 1 ?
+ brw_inst_src1_type(devinfo, inst) : 0;
+   enum brw_reg_type dst_type = brw_inst_dst_type(devinfo, inst);
+
+   unsigned dst_stride = STRIDE(brw_inst_dst_hstride(devinfo, inst));
+   bool dst_is_packed = is_packed(exec_size * dst_stride, exec_size, 
dst_stride);
+
+   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
+* Float Operations:
+*
+*"Indirect addressing on source is not supported when source and
+* destination data types are mixed float."
+*/
+   ERROR_IF((types_are_mixed_float(dst_type, src0_type) &&
+ brw_inst_src0_address_mode(devinfo, inst) != BRW_ADDRESS_DIRECT) 
||
+(num_sources > 1 &&
+ types_are_mixed_float(dst_type, src1_type) &&
+ brw_inst_src1_address_mode(devinfo, inst) != BRW_ADDRESS_DIRECT),
+"Indirect addressing on source is not supported when source and "
+"destination data types are mixed float");
+
+   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
+* Float Operations:
+*
+*"No SIMD16 in mixed mode when destination is f32. Instruction
+* execution size must be no more than 8."
+*/
+   ERROR_IF(exec_size > 8 && dst_type == BRW_REGISTER_TYPE_F,
+"Mixed float mode with 32-bit float destination is limited "
+"to SIMD8");
+
+   if (is_align16) {
+  /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
+   * Float Operations:
+   *
+   *   "In Align16 mode, when half float and float data types are mixed
+   *between source operands OR between