[Mesa-dev] [PATCH v4 19/28] i965/vec4: fix SIMD-width lowering for VEC4_OPCODE_FROM_DOUBLE

[Samuel Iglesias Gonsálvez]

On Thu, 2017-03-23 at 12:14 -0700, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
> 
> > On Wed, 2017-03-22 at 15:55 -0700, Francisco Jerez wrote:
> > > Samuel Iglesias Gonsálvez <siglesias at igalia.com> 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 <siglesias at igalia.com>
> > > > ---
> > > >  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 at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170324/f5ea7b1f/attachment.sig>