[Mesa-dev] [PATCH 02/19] intel/fs: Use groups for SIMD16 LINTERP on gen11+

[Kenneth Graunke]

On Monday, May 21, 2018 7:04:39 AM PDT Jason Ekstrand wrote:
> On May 20, 2018 23:45:27 Kenneth Graunke <kenneth at whitecape.org> wrote:
> 
> > On Friday, May 18, 2018 4:35:15 PM PDT Jason Ekstrand wrote:
> >> This is better than compression control because it naturally extends to
> >> SIMD32.
> >> ---
> >> src/intel/compiler/brw_fs_generator.cpp | 6 ++----
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/intel/compiler/brw_fs_generator.cpp 
> >> b/src/intel/compiler/brw_fs_generator.cpp
> >> index 0c050a7..f435f97 100644
> >> --- a/src/intel/compiler/brw_fs_generator.cpp
> >> +++ b/src/intel/compiler/brw_fs_generator.cpp
> >> @@ -795,16 +795,14 @@ fs_generator::generate_linterp(fs_inst *inst,
> >>   */
> >>  brw_inst_set_saturate(p->devinfo, i[0], false);
> >> } else {
> >> -         brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
> >> +         brw_set_default_group(p, inst->group);
> >>  i[0] = brw_MAD(p,            acc, dwR, offset(delta_x, 0), dwP);
> >>  i[1] = brw_MAD(p, offset(dst, 0), acc, offset(delta_x, 1), dwQ);
> >>
> >> -         brw_set_default_compression_control(p, BRW_COMPRESSION_2NDHALF);
> >> +         brw_set_default_group(p, inst->group + 8);
> >>  i[2] = brw_MAD(p,            acc, dwR, offset(delta_y, 0), dwP);
> >>  i[3] = brw_MAD(p, offset(dst, 1), acc, offset(delta_y, 1), dwQ);
> >>
> >> -         brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);
> >> -
> >>  brw_inst_set_cond_modifier(p->devinfo, i[1], inst->conditional_mod);
> >>  brw_inst_set_cond_modifier(p->devinfo, i[3], inst->conditional_mod);
> >
> > The last hunk you dropped is supposed to restore the original state, so
> > subsequent things continue working...I think you either want to change
> > it to brw_set_default_group(inst->group)...or push/pop the state around
> > this whole section...
> 
> It's not needed because no more instructions get emitted and we push/pop 
> around the giant generator switch anyway.  Still, I guess it doesn't hurt 
> to push/pop over extra time.

Mmm, yeah...but we do push/pop around most other sections that
temporarily whack something.  It's probably fine as is, but I think
it would be clearer to add it explicitly around this block.

With that change,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- 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/20180521/07f52b83/attachment.sig>