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 <n...@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

Matt Turner <matts...@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@arb_gpu_shader5@execution@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to