[Mesa-dev] [PATCH] i965/fs: Use UW-typed immediate in multiply inst.

[Ben Widawsky]

On Wed, Jun 03, 2015 at 01:09:50PM +0100, Neil Roberts wrote:
> Looks good to me. Thanks for fixing this. I guess I still have more to
> learn about the ISA.
> 
> However, should we not also fix the vec4 version? With that,
> 
> Reviewed-by: Neil Roberts <neil at linux.intel.com>
> 
> If we wanted to play safe and avoid the MUL, we could change it to this
> and still avoid having a temporary:
> 
>       /* addr = (((sampler << 8) | sampler) +
>        *         base_binding_table_index) & 0xfff
>        */
>       brw_SHL(p, addr, sampler_reg, brw_imm_ud(8u));
>       brw_OR(p, addr, addr, sampler_reg);
> 
> I just thought the MUL trick was more fun. I'm not sure which would be
> faster. The SHL+OR might take up fewer clock cycles but it also
> introduces a bubble because it is writing to a register and then
> immediately reading from it in the next instruction.
> 
> On the other hand it's probably really not worth worrying about
> optimising this bit of code because literally nothing is using it in
> shader-db. We can just go with whatever works.
> 
> Regards,
> - Neil
> 

I found this non-obvious myself but Matt explained it to me once. With few
exceptions all instructions take the same number of cycles. In general, less
instructions is always better.

> Matt Turner <mattst88 at gmail.com> writes:
> 
> > Some hardware reads only the low 16-bits even if the type is UD, but
> > other hardware like Cherryview can't handle this.
> >
> > Fixes spec at arb_gpu_shader5@execution at sampler_array_indexing@fs-simple on
> > Cherryview.
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 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 40a3db3..ff05b2a 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > @@ -788,7 +788,7 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src
> >        brw_set_default_access_mode(p, BRW_ALIGN_1);
> >  
> >        /* addr = ((sampler * 0x101) + base_binding_table_index) & 0xfff */
> > -      brw_MUL(p, addr, sampler_reg, brw_imm_ud(0x101));
> > +      brw_MUL(p, addr, sampler_reg, brw_imm_uw(0x101));
> >        if (base_binding_table_index)
> >           brw_ADD(p, addr, addr, brw_imm_ud(base_binding_table_index));
> >        brw_AND(p, addr, addr, brw_imm_ud(0xfff));
> > -- 
> > 2.3.6
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev