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