Re: [Mesa-dev] [PATCH v4 19/28] i965/vec4: fix SIMD-width lowering for VEC4_OPCODE_FROM_DOUBLE
On Thu, 2017-03-23 at 12:14 -0700, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez writes:
>
> > On Wed, 2017-03-22 at 15:55 -0700, Francisco Jerez wrote:
> > > Samuel Iglesias Gonsálvez writes:
> > >
> > > > Now the VEC4_OPCODE_FROM_DOUBLE's destination data is written
> > > > with
> > > > stride 2. We need to take into account this when doing the
> > > > split
> > > > so we don't overwrite data.
> > > >
> > >
> > > You should probably fix the destination type of your
> > > VEC4_OPCODE_FROM_DOUBLE instructions in PATCH 17 instead so you
> > > don't
> > > need to special-case VEC4_OPCODE_FROM_DOUBLE in this lowering
> > > pass.
> > >
> >
> > I don't think this would work. Do you mean to set the destination
> > type
> > to double, so horiz_offset() works fine here without any change?
> >
>
> Yes, double (or 64-bit integer) would be more appropriate than float,
> because the opcode writes 64 bit-wide components, so using F as
> destination type will inevitably confuse the optimizer.
>
Yes
> > Then in the generator's code for VEC4_OPCODE_FROM_DOUBLE, I would
> > need
> > to retype the destination to... I don't know to which type because
> > it
> > was lost.
> >
>
> It would make a lot more sense to rename these after the destination
> type (e.g. VEC4_OPCODE_TO_F32), since the conversion origin type is
> fully determined by the source type encoded in the IR instruction,
> but
> the destination type is not.
>
Right. I am going to write a patch to split this instruction in one per
type.
Sam
> > Sam
> >
> > > > Signed-off-by: Samuel Iglesias Gonsálvez
> > > > ---
> > > > src/intel/compiler/brw_vec4.cpp | 7 ++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/intel/compiler/brw_vec4.cpp
> > > > b/src/intel/compiler/brw_vec4.cpp
> > > > index b26f8035811..f4eea954404 100644
> > > > --- a/src/intel/compiler/brw_vec4.cpp
> > > > +++ b/src/intel/compiler/brw_vec4.cpp
> > > > @@ -2198,6 +2198,7 @@ vec4_visitor::lower_simd_width()
> > > > linst->group = channel_offset;
> > > > linst->size_written = size_written;
> > > >
> > > > + bool d2f_pass = (inst->opcode ==
> > > > VEC4_OPCODE_FROM_DOUBLE
> > > > && n > 0);
> > > > /* Compute split dst region */
> > > > dst_reg dst;
> > > > if (needs_temp) {
> > > > @@ -2212,7 +2213,11 @@ vec4_visitor::lower_simd_width()
> > > > inst->insert_before(block, copy);
> > > > }
> > > > } else {
> > > > -dst = horiz_offset(inst->dst, channel_offset);
> > > > +/* d2x conversion is done with a destination's
> > > > stride
> > > > of 2. We need
> > > > + * to take into account when splitting it.
> > > > + */
> > > > +unsigned stride = d2f_pass ? 2 : 1;
> > > > +dst = horiz_offset(inst->dst, stride *
> > > > channel_offset);
> > > > }
> > > > linst->dst = dst;
> > > >
> > > > --
> > > > 2.11.0
> > > >
> > > > ___
> > > > mesa-dev mailing list
> > > > mesa-dev@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 19/28] i965/vec4: fix SIMD-width lowering for VEC4_OPCODE_FROM_DOUBLE
Samuel Iglesias Gonsálvez writes:
> On Wed, 2017-03-22 at 15:55 -0700, Francisco Jerez wrote:
>> Samuel Iglesias Gonsálvez writes:
>>
>> > Now the VEC4_OPCODE_FROM_DOUBLE's destination data is written with
>> > stride 2. We need to take into account this when doing the split
>> > so we don't overwrite data.
>> >
>>
>> You should probably fix the destination type of your
>> VEC4_OPCODE_FROM_DOUBLE instructions in PATCH 17 instead so you don't
>> need to special-case VEC4_OPCODE_FROM_DOUBLE in this lowering pass.
>>
>
> I don't think this would work. Do you mean to set the destination type
> to double, so horiz_offset() works fine here without any change?
>
Yes, double (or 64-bit integer) would be more appropriate than float,
because the opcode writes 64 bit-wide components, so using F as
destination type will inevitably confuse the optimizer.
> Then in the generator's code for VEC4_OPCODE_FROM_DOUBLE, I would need
> to retype the destination to... I don't know to which type because it
> was lost.
>
It would make a lot more sense to rename these after the destination
type (e.g. VEC4_OPCODE_TO_F32), since the conversion origin type is
fully determined by the source type encoded in the IR instruction, but
the destination type is not.
> Sam
>
>> > Signed-off-by: Samuel Iglesias Gonsálvez
>> > ---
>> > src/intel/compiler/brw_vec4.cpp | 7 ++-
>> > 1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/intel/compiler/brw_vec4.cpp
>> > b/src/intel/compiler/brw_vec4.cpp
>> > index b26f8035811..f4eea954404 100644
>> > --- a/src/intel/compiler/brw_vec4.cpp
>> > +++ b/src/intel/compiler/brw_vec4.cpp
>> > @@ -2198,6 +2198,7 @@ vec4_visitor::lower_simd_width()
>> > linst->group = channel_offset;
>> > linst->size_written = size_written;
>> >
>> > + bool d2f_pass = (inst->opcode == VEC4_OPCODE_FROM_DOUBLE
>> > && n > 0);
>> > /* Compute split dst region */
>> > dst_reg dst;
>> > if (needs_temp) {
>> > @@ -2212,7 +2213,11 @@ vec4_visitor::lower_simd_width()
>> > inst->insert_before(block, copy);
>> > }
>> > } else {
>> > -dst = horiz_offset(inst->dst, channel_offset);
>> > +/* d2x conversion is done with a destination's stride
>> > of 2. We need
>> > + * to take into account when splitting it.
>> > + */
>> > +unsigned stride = d2f_pass ? 2 : 1;
>> > +dst = horiz_offset(inst->dst, stride *
>> > channel_offset);
>> > }
>> > linst->dst = dst;
>> >
>> > --
>> > 2.11.0
>> >
>> > ___
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 19/28] i965/vec4: fix SIMD-width lowering for VEC4_OPCODE_FROM_DOUBLE
On Wed, 2017-03-22 at 15:55 -0700, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez writes:
>
> > Now the VEC4_OPCODE_FROM_DOUBLE's destination data is written with
> > stride 2. We need to take into account this when doing the split
> > so we don't overwrite data.
> >
>
> You should probably fix the destination type of your
> VEC4_OPCODE_FROM_DOUBLE instructions in PATCH 17 instead so you don't
> need to special-case VEC4_OPCODE_FROM_DOUBLE in this lowering pass.
>
I don't think this would work. Do you mean to set the destination type
to double, so horiz_offset() works fine here without any change?
Then in the generator's code for VEC4_OPCODE_FROM_DOUBLE, I would need
to retype the destination to... I don't know to which type because it
was lost.
Sam
> > Signed-off-by: Samuel Iglesias Gonsálvez
> > ---
> > src/intel/compiler/brw_vec4.cpp | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/intel/compiler/brw_vec4.cpp
> > b/src/intel/compiler/brw_vec4.cpp
> > index b26f8035811..f4eea954404 100644
> > --- a/src/intel/compiler/brw_vec4.cpp
> > +++ b/src/intel/compiler/brw_vec4.cpp
> > @@ -2198,6 +2198,7 @@ vec4_visitor::lower_simd_width()
> > linst->group = channel_offset;
> > linst->size_written = size_written;
> >
> > + bool d2f_pass = (inst->opcode == VEC4_OPCODE_FROM_DOUBLE
> > && n > 0);
> > /* Compute split dst region */
> > dst_reg dst;
> > if (needs_temp) {
> > @@ -2212,7 +2213,11 @@ vec4_visitor::lower_simd_width()
> > inst->insert_before(block, copy);
> > }
> > } else {
> > -dst = horiz_offset(inst->dst, channel_offset);
> > +/* d2x conversion is done with a destination's stride
> > of 2. We need
> > + * to take into account when splitting it.
> > + */
> > +unsigned stride = d2f_pass ? 2 : 1;
> > +dst = horiz_offset(inst->dst, stride *
> > channel_offset);
> > }
> > linst->dst = dst;
> >
> > --
> > 2.11.0
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 19/28] i965/vec4: fix SIMD-width lowering for VEC4_OPCODE_FROM_DOUBLE
Samuel Iglesias Gonsálvez writes:
> Now the VEC4_OPCODE_FROM_DOUBLE's destination data is written with
> stride 2. We need to take into account this when doing the split
> so we don't overwrite data.
>
You should probably fix the destination type of your
VEC4_OPCODE_FROM_DOUBLE instructions in PATCH 17 instead so you don't
need to special-case VEC4_OPCODE_FROM_DOUBLE in this lowering pass.
> Signed-off-by: Samuel Iglesias Gonsálvez
> ---
> src/intel/compiler/brw_vec4.cpp | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
> index b26f8035811..f4eea954404 100644
> --- a/src/intel/compiler/brw_vec4.cpp
> +++ b/src/intel/compiler/brw_vec4.cpp
> @@ -2198,6 +2198,7 @@ vec4_visitor::lower_simd_width()
> linst->group = channel_offset;
> linst->size_written = size_written;
>
> + bool d2f_pass = (inst->opcode == VEC4_OPCODE_FROM_DOUBLE && n > 0);
> /* Compute split dst region */
> dst_reg dst;
> if (needs_temp) {
> @@ -2212,7 +2213,11 @@ vec4_visitor::lower_simd_width()
> inst->insert_before(block, copy);
> }
> } else {
> -dst = horiz_offset(inst->dst, channel_offset);
> +/* d2x conversion is done with a destination's stride of 2. We
> need
> + * to take into account when splitting it.
> + */
> +unsigned stride = d2f_pass ? 2 : 1;
> +dst = horiz_offset(inst->dst, stride * channel_offset);
> }
> linst->dst = dst;
>
> --
> 2.11.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4 19/28] i965/vec4: fix SIMD-width lowering for VEC4_OPCODE_FROM_DOUBLE
Now the VEC4_OPCODE_FROM_DOUBLE's destination data is written with
stride 2. We need to take into account this when doing the split
so we don't overwrite data.
Signed-off-by: Samuel Iglesias Gonsálvez
---
src/intel/compiler/brw_vec4.cpp | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
index b26f8035811..f4eea954404 100644
--- a/src/intel/compiler/brw_vec4.cpp
+++ b/src/intel/compiler/brw_vec4.cpp
@@ -2198,6 +2198,7 @@ vec4_visitor::lower_simd_width()
linst->group = channel_offset;
linst->size_written = size_written;
+ bool d2f_pass = (inst->opcode == VEC4_OPCODE_FROM_DOUBLE && n > 0);
/* Compute split dst region */
dst_reg dst;
if (needs_temp) {
@@ -2212,7 +2213,11 @@ vec4_visitor::lower_simd_width()
inst->insert_before(block, copy);
}
} else {
-dst = horiz_offset(inst->dst, channel_offset);
+/* d2x conversion is done with a destination's stride of 2. We need
+ * to take into account when splitting it.
+ */
+unsigned stride = d2f_pass ? 2 : 1;
+dst = horiz_offset(inst->dst, stride * channel_offset);
}
linst->dst = dst;
--
2.11.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
