Re: [Mesa-dev] [PATCH v3 19/42] intel/compiler: don't compact 3-src instructions with Src1Type or Src2Type bits
On Tue, Jan 15, 2019 at 5:55 AM Iago Toral Quiroga wrote: > > We are now using these bits, so don't assert that they are not set, just > avoid compaction in that case. > > Reviewed-by: Topi Pohjolainen > --- > src/intel/compiler/brw_eu_compact.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_eu_compact.c > b/src/intel/compiler/brw_eu_compact.c > index ae14ef10ec0..20fed254331 100644 > --- a/src/intel/compiler/brw_eu_compact.c > +++ b/src/intel/compiler/brw_eu_compact.c > @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct gen_device_info > *devinfo, >assert(!brw_inst_bits(src, 127, 126) && > !brw_inst_bits(src, 105, 105) && > !brw_inst_bits(src, 84, 84) && > - !brw_inst_bits(src, 36, 35) && > !brw_inst_bits(src, 7, 7)); > + > + /* Src1Type and Src2Type, used for mixed-precision floating point */ > + if (brw_inst_bits(src, 36, 35)) > + return true; > } These bits are used on SKL+ and CHV (which is handled immediately above this hunk), so this is only modifying BDW. All looks correct to me. Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 19/42] intel/compiler: don't compact 3-src instructions with Src1Type or Src2Type bits
On January 18, 2019 04:12:44 Iago Toral wrote: On Thu, 2019-01-17 at 14:14 -0600, Jason Ekstrand wrote: On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga wrote: We are now using these bits, so don't assert that they are not set, just avoid compaction in that case. Reviewed-by: Topi Pohjolainen --- src/intel/compiler/brw_eu_compact.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_eu_compact.c b/src/intel/compiler/brw_eu_compact.c index ae14ef10ec0..20fed254331 100644 --- a/src/intel/compiler/brw_eu_compact.c +++ b/src/intel/compiler/brw_eu_compact.c @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct gen_device_info *devinfo, assert(!brw_inst_bits(src, 127, 126) && !brw_inst_bits(src, 105, 105) && !brw_inst_bits(src, 84, 84) && - !brw_inst_bits(src, 36, 35) && !brw_inst_bits(src, 7, 7)); + + /* Src1Type and Src2Type, used for mixed-precision floating point */ + if (brw_inst_bits(src, 36, 35)) + return true; You're only doing this in the broadwell case. What about SKL+ and CHV? Can we compact mixed-precision stuff there? Looks like maybe we can but there should be at least something in the commit message about that. In these platforms compaction is possible in some cases and set_3src_control_index() takes care of this by including these bits in a tablre lookup for accepted combinations. I can add this to the commit message: "We are now using these bits, so don't assert that they are not set. In gen8, if these bits are set compaction is not possible. On gen9 and CHV platforms set_3src_control_index() checks these bits (and others) against a table to validate if the particular bit combination is eligible for compaction or not." With that said, if I am reading this correctly, it looks like all entries in gen8_3src_control_index_table that allow compaction require these bits to be zero at present, so I guess that right now we could also just extend the check I am adding here for BDW to other platforms, however, I guess that relying on the array with accepted bit combinations for compaction is more reliable should any future platforms allow for more combinations. Sounds good. With that commit message update, r-b me. } return false; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 19/42] intel/compiler: don't compact 3-src instructions with Src1Type or Src2Type bits
On Thu, 2019-01-17 at 14:14 -0600, Jason Ekstrand wrote: > On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga > wrote: > > We are now using these bits, so don't assert that they are not set, > > just > > > > avoid compaction in that case. > > > > > > > > Reviewed-by: Topi Pohjolainen > > > > --- > > > > src/intel/compiler/brw_eu_compact.c | 5 - > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/intel/compiler/brw_eu_compact.c > > b/src/intel/compiler/brw_eu_compact.c > > > > index ae14ef10ec0..20fed254331 100644 > > > > --- a/src/intel/compiler/brw_eu_compact.c > > > > +++ b/src/intel/compiler/brw_eu_compact.c > > > > @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct > > gen_device_info *devinfo, > > > >assert(!brw_inst_bits(src, 127, 126) && > > > > !brw_inst_bits(src, 105, 105) && > > > > !brw_inst_bits(src, 84, 84) && > > > > - !brw_inst_bits(src, 36, 35) && > > > > !brw_inst_bits(src, 7, 7)); > > > > + > > > > + /* Src1Type and Src2Type, used for mixed-precision floating > > point */ > > > > + if (brw_inst_bits(src, 36, 35)) > > > > + return true; > > You're only doing this in the broadwell case. What about SKL+ and > CHV? Can we compact mixed-precision stuff there? Looks like maybe > we can but there should be at least something in the commit message > about that. In these platforms compaction is possible in some cases and set_3src_control_index() takescare of this by including these bits in a tablre lookup for accepted combinations. I can add thisto the commit message: "We are now using these bits, so don't assert that they are not set. In gen8, if these bits are setcompaction is not possible. On gen9 and CHV platforms set_3src_control_index() checks thesebits (and others) against a table to validate if the particular bit combination is eligible forcompaction or not." With that said, if I am reading this correctly, it looks like all entries in gen8_3src_control_index_table that allow compaction require these bits to be zero at present, so I guess that right now we could also just extend the check I am adding here for BDW to other platforms, however, I guess that relying on the array with accepted bit combinations for compaction is more reliable should any future platforms allow for more combinations. > > } > > > > > > > > return false; > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 19/42] intel/compiler: don't compact 3-src instructions with Src1Type or Src2Type bits
On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga wrote: > We are now using these bits, so don't assert that they are not set, just > avoid compaction in that case. > > Reviewed-by: Topi Pohjolainen > --- > src/intel/compiler/brw_eu_compact.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_eu_compact.c > b/src/intel/compiler/brw_eu_compact.c > index ae14ef10ec0..20fed254331 100644 > --- a/src/intel/compiler/brw_eu_compact.c > +++ b/src/intel/compiler/brw_eu_compact.c > @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct gen_device_info > *devinfo, >assert(!brw_inst_bits(src, 127, 126) && > !brw_inst_bits(src, 105, 105) && > !brw_inst_bits(src, 84, 84) && > - !brw_inst_bits(src, 36, 35) && > !brw_inst_bits(src, 7, 7)); > + > + /* Src1Type and Src2Type, used for mixed-precision floating point */ > + if (brw_inst_bits(src, 36, 35)) > + return true; > You're only doing this in the broadwell case. What about SKL+ and CHV? Can we compact mixed-precision stuff there? Looks like maybe we can but there should be at least something in the commit message about that. > } > > return false; > -- > 2.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 19/42] intel/compiler: don't compact 3-src instructions with Src1Type or Src2Type bits
We are now using these bits, so don't assert that they are not set, just avoid compaction in that case. Reviewed-by: Topi Pohjolainen --- src/intel/compiler/brw_eu_compact.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_eu_compact.c b/src/intel/compiler/brw_eu_compact.c index ae14ef10ec0..20fed254331 100644 --- a/src/intel/compiler/brw_eu_compact.c +++ b/src/intel/compiler/brw_eu_compact.c @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct gen_device_info *devinfo, assert(!brw_inst_bits(src, 127, 126) && !brw_inst_bits(src, 105, 105) && !brw_inst_bits(src, 84, 84) && - !brw_inst_bits(src, 36, 35) && !brw_inst_bits(src, 7, 7)); + + /* Src1Type and Src2Type, used for mixed-precision floating point */ + if (brw_inst_bits(src, 36, 35)) + return true; } return false; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
