On Mon, 2017-01-09 at 16:18 -0800, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes:
> 
> > From: Iago Toral Quiroga <ito...@igalia.com>
> > 
> > It seems to use 1 channel por DF, just like later hardware. The
> > docs say things
> > like:
> > 
> > "Each DF operand uses a pair of channels and all masking  and
> > swizzling
> >  should be adjusted appropriately."
> > 
> > "In Align16, all regioning parameters must use the syntax of a pair
> > of packed
> >  floats, including channel selects and channel enables."
> > 
> > In these cases, masking and channel select/enables seem to refere
> > only to
> > Align16's swizzle and writemasks respectively, not to execution
> > masking.
> > 
> > In any case, this means that execution masking controls and
> > execsize go
> > in different units and we need to adjust the assertion on the
> > relation
> > between the two accordingly.
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > index 90ee7c1..ac2d8ad 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > @@ -1646,7 +1646,11 @@ fs_generator::generate_code(const cfg_t
> > *cfg, int dispatch_width)
> >        brw_set_default_exec_size(p, cvt(inst->exec_size) - 1);
> >  
> >        assert(inst->force_writemask_all || inst->exec_size >= 4);
> > -      assert(inst->force_writemask_all || inst->group % inst-
> > >exec_size == 0);
> > +      assert(inst->force_writemask_all ||
> > +             ((devinfo->gen != 7 || devinfo->is_haswell) &&
> > +              inst->group % inst->exec_size == 0) ||
> > +             ((devinfo->gen == 7 && !devinfo->is_haswell) &&
> > +              (2 * inst->group) % inst->exec_size == 0));
> 
> NAK, exec size doubling happens at the codegen level so the IR group
> and
> exec_size controls should still be expressed in the same units.
> 

Actually the patch is right because the exec size doubling already
happened at this point, so this is fixing the difference in units.

One alternative to this patch is to move the exec_size doubling code to
just after this assert, so we can discard this patch.

What do you think?

Sam

> >        assert(inst->base_mrf + inst->mlen <= BRW_MAX_MRF(devinfo-
> > >gen));
> >        assert(inst->mlen <= BRW_MAX_MSG_LENGTH);
> >  
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Attachment: 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

Reply via email to