Re: [Mesa-dev] [PATCH 28/59] intel/compiler: set correct precision fields for 3-source float instructions
On Tue, 2018-12-11 at 10:01 +0200, Pohjolainen, Topi wrote:
> On Fri, Dec 07, 2018 at 12:08:15PM -0600, Jason Ekstrand wrote:
> > On Wed, Dec 5, 2018 at 7:14 AM Pohjolainen, Topi <
> > topi.pohjolai...@gmail.com>
> > wrote:
> >
> > > On Wed, Dec 05, 2018 at 02:04:16PM +0100, Iago Toral wrote:
> > > > On Wed, 2018-12-05 at 14:58 +0200, Pohjolainen, Topi wrote:
> > > > > On Tue, Dec 04, 2018 at 08:16:52AM +0100, Iago Toral Quiroga
> > > > > wrote:
> > > > > > Source0 and Destination extract the floating-point
> > > > > > precision
> > > > > > automatically
> > > > > > from the SrcType and DstType instruction fields
> > > > > > respectively when
> > > > > > they are
> > > > > > set to types :F or :HF. For Source1 and Source2 operands,
> > > > > > we use
> > > > > > the new
> > > > > > 1-bit fields Src1Type and Src2Type, where 0 means normal
> > > > > > precision
> > > > > > and 1
> > > > > > means half-precision. Since we always use the type of the
> > > > > > destination for
> > > > > > all operands when we emit 3-source instructions, we only
> > > > > > need set
> > > > > > Src1Type
> > > > > > and Src2Type to 1 when we are emitting a half-precision
> > > > > > instruction.
> > > > > > ---
> > > > > > src/intel/compiler/brw_eu_emit.c | 5 +
> > > > > > 1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/src/intel/compiler/brw_eu_emit.c
> > > > > > b/src/intel/compiler/brw_eu_emit.c
> > > > > > index 2c9fc9a5c7c..66edfb43baf 100644
> > > > > > --- a/src/intel/compiler/brw_eu_emit.c
> > > > > > +++ b/src/intel/compiler/brw_eu_emit.c
> > > > > > @@ -801,6 +801,11 @@ brw_alu3(struct brw_codegen *p,
> > > > > > unsigned
> > > > > > opcode, struct brw_reg dest,
> > > > > >*/
> > > > > > brw_inst_set_3src_a16_src_type(devinfo, inst,
> > > > > > dest.type);
> > > > > > brw_inst_set_3src_a16_dst_type(devinfo, inst,
> > > > > > dest.type);
> > > > > > +
> > > > > > + if (devinfo->gen >= 8 && dest.type ==
> > > > > > BRW_REGISTER_TYPE_HF) {
> > > > > > +brw_inst_set_3src_a16_src1_type(devinfo, inst,
> > > > > > 1);
> > > > > > +brw_inst_set_3src_a16_src2_type(devinfo, inst,
> > > > > > 1);
> > > > > > + }
> > > > >
> > > > > I had similar patch which prepares for mixed mode (useful for
> > > > > linterp
> > > > > with
> > > > > 32-bit input varyings):
> > > > >
> > > > > /* From the Bspec: Instruction types
> > > > > *
> > > > > * Three source instructions can use operands with
> > > > > mixed-
> > > > > mode
> > > > > * precision. When SrcType field is set to :f or :hf
> > > > > it
> > > > > defines
> > > > > * precision for source 0 only, and fields Src1Type
> > > > > and
> > > > > Src2Type
> > > > > * define precision for other source operands:
> > > > > *
> > > > > * 0b = :f. Single precision Float (32-bit).
> > > > > * 1b = :hf. Half precision Float (16-bit).
> > > > > */
> > > > > if (src1.type == BRW_REGISTER_TYPE_HF)
> > > > > brw_inst_set_3src_src1_type(devinfo, inst, 1);
> > > > >
> > > > > if (src2.type == BRW_REGISTER_TYPE_HF)
> > > > > brw_inst_set_3src_src2_type(devinfo, inst, 1);
> > > > >
> > > > > How would you feel about that? (Direct cut-paste and the
> > > > > helpers have
> > > > > different name).'
> >
> > Yeah, let's not base source precisions on destination types. I
> > like Topi's
> > version better.
>
> I realized that one also needs to relax some asserts accordingly:
>
>
https://gitlab.freedesktop.org/tpohjola/mesa/commit/80b2483079ea8f5b031c1a397173c7a762d307cc
>
> I guess one doesn't hit these cases before glsl mediump support
> though.
Right, on the spir-v path we always have operands of the same type, so
this is not a problem right now.
Iago
> >
> >
> > > >
> > > > Sure, if we are planning to use mixed mode in the future this
> > > > makes
> > > > more sense. Thanks!
> > >
> > > Nice!
> > >
> > > Reviewed-by: Topi Pohjolainen
> > > ___
> > > 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
Re: [Mesa-dev] [PATCH 28/59] intel/compiler: set correct precision fields for 3-source float instructions
On Fri, Dec 07, 2018 at 12:08:15PM -0600, Jason Ekstrand wrote:
> On Wed, Dec 5, 2018 at 7:14 AM Pohjolainen, Topi
> wrote:
>
> > On Wed, Dec 05, 2018 at 02:04:16PM +0100, Iago Toral wrote:
> > > On Wed, 2018-12-05 at 14:58 +0200, Pohjolainen, Topi wrote:
> > > > On Tue, Dec 04, 2018 at 08:16:52AM +0100, Iago Toral Quiroga wrote:
> > > > > Source0 and Destination extract the floating-point precision
> > > > > automatically
> > > > > from the SrcType and DstType instruction fields respectively when
> > > > > they are
> > > > > set to types :F or :HF. For Source1 and Source2 operands, we use
> > > > > the new
> > > > > 1-bit fields Src1Type and Src2Type, where 0 means normal precision
> > > > > and 1
> > > > > means half-precision. Since we always use the type of the
> > > > > destination for
> > > > > all operands when we emit 3-source instructions, we only need set
> > > > > Src1Type
> > > > > and Src2Type to 1 when we are emitting a half-precision
> > > > > instruction.
> > > > > ---
> > > > > src/intel/compiler/brw_eu_emit.c | 5 +
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/src/intel/compiler/brw_eu_emit.c
> > > > > b/src/intel/compiler/brw_eu_emit.c
> > > > > index 2c9fc9a5c7c..66edfb43baf 100644
> > > > > --- a/src/intel/compiler/brw_eu_emit.c
> > > > > +++ b/src/intel/compiler/brw_eu_emit.c
> > > > > @@ -801,6 +801,11 @@ brw_alu3(struct brw_codegen *p, unsigned
> > > > > opcode, struct brw_reg dest,
> > > > >*/
> > > > > brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);
> > > > > brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);
> > > > > +
> > > > > + if (devinfo->gen >= 8 && dest.type ==
> > > > > BRW_REGISTER_TYPE_HF) {
> > > > > +brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
> > > > > +brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);
> > > > > + }
> > > >
> > > > I had similar patch which prepares for mixed mode (useful for linterp
> > > > with
> > > > 32-bit input varyings):
> > > >
> > > > /* From the Bspec: Instruction types
> > > > *
> > > > * Three source instructions can use operands with mixed-
> > > > mode
> > > > * precision. When SrcType field is set to :f or :hf it
> > > > defines
> > > > * precision for source 0 only, and fields Src1Type and
> > > > Src2Type
> > > > * define precision for other source operands:
> > > > *
> > > > * 0b = :f. Single precision Float (32-bit).
> > > > * 1b = :hf. Half precision Float (16-bit).
> > > > */
> > > > if (src1.type == BRW_REGISTER_TYPE_HF)
> > > > brw_inst_set_3src_src1_type(devinfo, inst, 1);
> > > >
> > > > if (src2.type == BRW_REGISTER_TYPE_HF)
> > > > brw_inst_set_3src_src2_type(devinfo, inst, 1);
> > > >
> > > > How would you feel about that? (Direct cut-paste and the helpers have
> > > > different name).'
> >
>
> Yeah, let's not base source precisions on destination types. I like Topi's
> version better.
I realized that one also needs to relax some asserts accordingly:
https://gitlab.freedesktop.org/tpohjola/mesa/commit/80b2483079ea8f5b031c1a397173c7a762d307cc
I guess one doesn't hit these cases before glsl mediump support though.
>
>
> > >
> > > Sure, if we are planning to use mixed mode in the future this makes
> > > more sense. Thanks!
> >
> > Nice!
> >
> > Reviewed-by: Topi Pohjolainen
> > ___
> > 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
Re: [Mesa-dev] [PATCH 28/59] intel/compiler: set correct precision fields for 3-source float instructions
On Wed, Dec 5, 2018 at 7:14 AM Pohjolainen, Topi
wrote:
> On Wed, Dec 05, 2018 at 02:04:16PM +0100, Iago Toral wrote:
> > On Wed, 2018-12-05 at 14:58 +0200, Pohjolainen, Topi wrote:
> > > On Tue, Dec 04, 2018 at 08:16:52AM +0100, Iago Toral Quiroga wrote:
> > > > Source0 and Destination extract the floating-point precision
> > > > automatically
> > > > from the SrcType and DstType instruction fields respectively when
> > > > they are
> > > > set to types :F or :HF. For Source1 and Source2 operands, we use
> > > > the new
> > > > 1-bit fields Src1Type and Src2Type, where 0 means normal precision
> > > > and 1
> > > > means half-precision. Since we always use the type of the
> > > > destination for
> > > > all operands when we emit 3-source instructions, we only need set
> > > > Src1Type
> > > > and Src2Type to 1 when we are emitting a half-precision
> > > > instruction.
> > > > ---
> > > > src/intel/compiler/brw_eu_emit.c | 5 +
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/src/intel/compiler/brw_eu_emit.c
> > > > b/src/intel/compiler/brw_eu_emit.c
> > > > index 2c9fc9a5c7c..66edfb43baf 100644
> > > > --- a/src/intel/compiler/brw_eu_emit.c
> > > > +++ b/src/intel/compiler/brw_eu_emit.c
> > > > @@ -801,6 +801,11 @@ brw_alu3(struct brw_codegen *p, unsigned
> > > > opcode, struct brw_reg dest,
> > > >*/
> > > > brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);
> > > > brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);
> > > > +
> > > > + if (devinfo->gen >= 8 && dest.type ==
> > > > BRW_REGISTER_TYPE_HF) {
> > > > +brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
> > > > +brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);
> > > > + }
> > >
> > > I had similar patch which prepares for mixed mode (useful for linterp
> > > with
> > > 32-bit input varyings):
> > >
> > > /* From the Bspec: Instruction types
> > > *
> > > * Three source instructions can use operands with mixed-
> > > mode
> > > * precision. When SrcType field is set to :f or :hf it
> > > defines
> > > * precision for source 0 only, and fields Src1Type and
> > > Src2Type
> > > * define precision for other source operands:
> > > *
> > > * 0b = :f. Single precision Float (32-bit).
> > > * 1b = :hf. Half precision Float (16-bit).
> > > */
> > > if (src1.type == BRW_REGISTER_TYPE_HF)
> > > brw_inst_set_3src_src1_type(devinfo, inst, 1);
> > >
> > > if (src2.type == BRW_REGISTER_TYPE_HF)
> > > brw_inst_set_3src_src2_type(devinfo, inst, 1);
> > >
> > > How would you feel about that? (Direct cut-paste and the helpers have
> > > different name).'
>
Yeah, let's not base source precisions on destination types. I like Topi's
version better.
> >
> > Sure, if we are planning to use mixed mode in the future this makes
> > more sense. Thanks!
>
> Nice!
>
> Reviewed-by: Topi Pohjolainen
> ___
> 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
Re: [Mesa-dev] [PATCH 28/59] intel/compiler: set correct precision fields for 3-source float instructions
On Wed, Dec 05, 2018 at 02:04:16PM +0100, Iago Toral wrote:
> On Wed, 2018-12-05 at 14:58 +0200, Pohjolainen, Topi wrote:
> > On Tue, Dec 04, 2018 at 08:16:52AM +0100, Iago Toral Quiroga wrote:
> > > Source0 and Destination extract the floating-point precision
> > > automatically
> > > from the SrcType and DstType instruction fields respectively when
> > > they are
> > > set to types :F or :HF. For Source1 and Source2 operands, we use
> > > the new
> > > 1-bit fields Src1Type and Src2Type, where 0 means normal precision
> > > and 1
> > > means half-precision. Since we always use the type of the
> > > destination for
> > > all operands when we emit 3-source instructions, we only need set
> > > Src1Type
> > > and Src2Type to 1 when we are emitting a half-precision
> > > instruction.
> > > ---
> > > src/intel/compiler/brw_eu_emit.c | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/src/intel/compiler/brw_eu_emit.c
> > > b/src/intel/compiler/brw_eu_emit.c
> > > index 2c9fc9a5c7c..66edfb43baf 100644
> > > --- a/src/intel/compiler/brw_eu_emit.c
> > > +++ b/src/intel/compiler/brw_eu_emit.c
> > > @@ -801,6 +801,11 @@ brw_alu3(struct brw_codegen *p, unsigned
> > > opcode, struct brw_reg dest,
> > >*/
> > > brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);
> > > brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);
> > > +
> > > + if (devinfo->gen >= 8 && dest.type ==
> > > BRW_REGISTER_TYPE_HF) {
> > > +brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
> > > +brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);
> > > + }
> >
> > I had similar patch which prepares for mixed mode (useful for linterp
> > with
> > 32-bit input varyings):
> >
> > /* From the Bspec: Instruction types
> > *
> > * Three source instructions can use operands with mixed-
> > mode
> > * precision. When SrcType field is set to :f or :hf it
> > defines
> > * precision for source 0 only, and fields Src1Type and
> > Src2Type
> > * define precision for other source operands:
> > *
> > * 0b = :f. Single precision Float (32-bit).
> > * 1b = :hf. Half precision Float (16-bit).
> > */
> > if (src1.type == BRW_REGISTER_TYPE_HF)
> > brw_inst_set_3src_src1_type(devinfo, inst, 1);
> >
> > if (src2.type == BRW_REGISTER_TYPE_HF)
> > brw_inst_set_3src_src2_type(devinfo, inst, 1);
> >
> > How would you feel about that? (Direct cut-paste and the helpers have
> > different name).
>
> Sure, if we are planning to use mixed mode in the future this makes
> more sense. Thanks!
Nice!
Reviewed-by: Topi Pohjolainen
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 28/59] intel/compiler: set correct precision fields for 3-source float instructions
On Wed, 2018-12-05 at 14:58 +0200, Pohjolainen, Topi wrote:
> On Tue, Dec 04, 2018 at 08:16:52AM +0100, Iago Toral Quiroga wrote:
> > Source0 and Destination extract the floating-point precision
> > automatically
> > from the SrcType and DstType instruction fields respectively when
> > they are
> > set to types :F or :HF. For Source1 and Source2 operands, we use
> > the new
> > 1-bit fields Src1Type and Src2Type, where 0 means normal precision
> > and 1
> > means half-precision. Since we always use the type of the
> > destination for
> > all operands when we emit 3-source instructions, we only need set
> > Src1Type
> > and Src2Type to 1 when we are emitting a half-precision
> > instruction.
> > ---
> > src/intel/compiler/brw_eu_emit.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/src/intel/compiler/brw_eu_emit.c
> > b/src/intel/compiler/brw_eu_emit.c
> > index 2c9fc9a5c7c..66edfb43baf 100644
> > --- a/src/intel/compiler/brw_eu_emit.c
> > +++ b/src/intel/compiler/brw_eu_emit.c
> > @@ -801,6 +801,11 @@ brw_alu3(struct brw_codegen *p, unsigned
> > opcode, struct brw_reg dest,
> >*/
> > brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);
> > brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);
> > +
> > + if (devinfo->gen >= 8 && dest.type ==
> > BRW_REGISTER_TYPE_HF) {
> > +brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
> > +brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);
> > + }
>
> I had similar patch which prepares for mixed mode (useful for linterp
> with
> 32-bit input varyings):
>
> /* From the Bspec: Instruction types
> *
> * Three source instructions can use operands with mixed-
> mode
> * precision. When SrcType field is set to :f or :hf it
> defines
> * precision for source 0 only, and fields Src1Type and
> Src2Type
> * define precision for other source operands:
> *
> * 0b = :f. Single precision Float (32-bit).
> * 1b = :hf. Half precision Float (16-bit).
> */
> if (src1.type == BRW_REGISTER_TYPE_HF)
> brw_inst_set_3src_src1_type(devinfo, inst, 1);
>
> if (src2.type == BRW_REGISTER_TYPE_HF)
> brw_inst_set_3src_src2_type(devinfo, inst, 1);
>
> How would you feel about that? (Direct cut-paste and the helpers have
> different name).
Sure, if we are planning to use mixed mode in the future this makes
more sense. Thanks!
> >}
> > }
> >
> > --
> > 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
Re: [Mesa-dev] [PATCH 28/59] intel/compiler: set correct precision fields for 3-source float instructions
On Tue, Dec 04, 2018 at 08:16:52AM +0100, Iago Toral Quiroga wrote:
> Source0 and Destination extract the floating-point precision automatically
> from the SrcType and DstType instruction fields respectively when they are
> set to types :F or :HF. For Source1 and Source2 operands, we use the new
> 1-bit fields Src1Type and Src2Type, where 0 means normal precision and 1
> means half-precision. Since we always use the type of the destination for
> all operands when we emit 3-source instructions, we only need set Src1Type
> and Src2Type to 1 when we are emitting a half-precision instruction.
> ---
> src/intel/compiler/brw_eu_emit.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/intel/compiler/brw_eu_emit.c
> b/src/intel/compiler/brw_eu_emit.c
> index 2c9fc9a5c7c..66edfb43baf 100644
> --- a/src/intel/compiler/brw_eu_emit.c
> +++ b/src/intel/compiler/brw_eu_emit.c
> @@ -801,6 +801,11 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, struct
> brw_reg dest,
>*/
> brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);
> brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);
> +
> + if (devinfo->gen >= 8 && dest.type == BRW_REGISTER_TYPE_HF) {
> +brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
> +brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);
> + }
I had similar patch which prepares for mixed mode (useful for linterp with
32-bit input varyings):
/* From the Bspec: Instruction types
*
* Three source instructions can use operands with mixed-mode
* precision. When SrcType field is set to :f or :hf it defines
* precision for source 0 only, and fields Src1Type and Src2Type
* define precision for other source operands:
*
* 0b = :f. Single precision Float (32-bit).
* 1b = :hf. Half precision Float (16-bit).
*/
if (src1.type == BRW_REGISTER_TYPE_HF)
brw_inst_set_3src_src1_type(devinfo, inst, 1);
if (src2.type == BRW_REGISTER_TYPE_HF)
brw_inst_set_3src_src2_type(devinfo, inst, 1);
How would you feel about that? (Direct cut-paste and the helpers have
different name).
>}
> }
>
> --
> 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 28/59] intel/compiler: set correct precision fields for 3-source float instructions
Source0 and Destination extract the floating-point precision automatically
from the SrcType and DstType instruction fields respectively when they are
set to types :F or :HF. For Source1 and Source2 operands, we use the new
1-bit fields Src1Type and Src2Type, where 0 means normal precision and 1
means half-precision. Since we always use the type of the destination for
all operands when we emit 3-source instructions, we only need set Src1Type
and Src2Type to 1 when we are emitting a half-precision instruction.
---
src/intel/compiler/brw_eu_emit.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c
index 2c9fc9a5c7c..66edfb43baf 100644
--- a/src/intel/compiler/brw_eu_emit.c
+++ b/src/intel/compiler/brw_eu_emit.c
@@ -801,6 +801,11 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, struct
brw_reg dest,
*/
brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);
brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);
+
+ if (devinfo->gen >= 8 && dest.type == BRW_REGISTER_TYPE_HF) {
+brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
+brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);
+ }
}
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
