On Tue, 2017-01-17 at 13:26 -0800, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes:
>
> > From: "Juan A. Suarez Romero" <jasua...@igalia.com>
> >
> > When converting a DF to F, we set dst stride to 2, to fulfil
> > alignment
> > restrictions.
> >
> > But in IVB/BYT, this is not necessary, as each DF conversion
> > already
> > writes 2 F, the first one the real value, and the second one a 0.
> > That
> > is, IVB/BYT already set stride = 2 implicitly, so we must set it to
> > 1
> > explicitly to avoid ending up with stride = 4.
> >
> > v2:
> > - Fix typo (Matt)
> > ---
> > src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > index 45881e3ec95..487f2e90224 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > @@ -1629,6 +1629,16 @@ fs_generator::generate_code(const cfg_t
> > *cfg, int dispatch_width)
> > inst->src[i].type != BRW_REGISTER_TYPE_UD ||
> > !inst->src[i].negate);
> > }
> > + /* When converting from DF->F, we set destination's stride
> > as 2 as an
> > + * alignment requirement. But in IVB/BYT, each DF implicitly
> > writes 2 F,
> > + * being the first one the converted value. So we don't need
> > to
> > + * explicitly set stride 2, but 1.
> > + */
> > + if (devinfo->gen == 7 && !devinfo->is_haswell &&
> > + type_sz(inst->src[0].type) > type_sz(inst->dst.type)) {
>
> This should be exec_type_size(inst) rather than
> type_sz(inst->src[0].type).
> Actually it is going to be the same as this is going to catch only DF -> 32-bit data type conversions. But yeah, you are right. > > + assert(inst->dst.stride == 2 || inst->dst.stride == 1); > > + inst->dst.stride = 1; > > + } > > This is modifying the IR, please don't. > Right, I am going to do the change in brw_eu_emit.c inside the brw_MOV() function that Matt added in other patch and also when emitting the MOV indirect. > Also I don't think the above has the same semantics as a destination > region with stride 2... AFAIUI IVB will just write garbage into the > odd > channels when the destination type is narrower than a DF which is > really > not what a strided move is supposed to do. If that's the case it > would > probably be safer to add a new F64TO32 virtual opcode for type > conversions and assert(inst->dst.stride == 1) here... > I think adding a virtual opcode for this change is likely too much. I will keep the aforementioned changes. However, I don't have a strong opinion against it: if you prefer the virtual opcode, we can add it now or even later as a follow-up patch. Sam > > dst = brw_reg_from_fs_reg(compiler->devinfo, inst, > > &inst->dst, compressed); > > > > -- > > 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
