<p dir="ltr"><br>
On Sep 15, 2015 6:39 AM, "Francisco Jerez" <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote:<br>
><br>
> Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
><br>
> > In certain conditions, we have to do bounds-checking in the shader for<br>
> > image_load_store.  The way this works for image loads is that we do the<br>
> > load anyway and then emit a series of slecects, one per component, that<br>
><br>
> Strictly speaking the load is predicated so it's not really done for out<br>
> of bounds coordinates, with that sentence clarified or removed (and the<br>
> typo fixed in "slecects") this patch is:<br>
><br>
> Reviewed-by: Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>><br>
><br>
> > gives us 0 or the loaded value depending on whether or not you're in<br>
> > bounds.  However, we were hard-coding 4 components which may not be<br>
> > correct.  Instead, we should be using size which is the number of<br>
> > components read.<br>
> ><br>
> > Cc: Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>><br>
><br>
> Although this is unlikely to fix any preexisting bugs (because the<br>
> result of the extra SELs would have been left unused and they would have<br>
> been dead-code-eliminated later on anyway), it shouldn't hurt to CC this<br>
> to mesa-stable too, if you like.</p>
<p dir="ltr">Sure. Really, its just to make it pass the validation pass so its not a big deal.  Still, there is a theoretical bug there so I'll do that.</p>
<p dir="ltr">> > ---<br>
> >  src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp | 2 +-<br>
> >  1 file changed, 1 insertion(+), 1 deletion(-)<br>
> ><br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp<br>
> > index 727e8d1..88f22fa 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp<br>
> > @@ -905,7 +905,7 @@ namespace brw {<br>
> >              tmp = emit_untyped_read(bld, image, laddr, 1, size, pred);<br>
> ><br>
> >              /* An out of bounds surface access should give zero as result. */<br>
> > -            for (unsigned c = 0; c < 4; ++c)<br>
> > +            for (unsigned c = 0; c < size; ++c)<br>
> >                 set_predicate(pred, bld.SEL(offset(tmp, bld, c),<br>
> >                                             offset(tmp, bld, c), fs_reg(0)));<br>
> >           }<br>
> > --<br>
> > 2.5.0.400.gff86faf<br>
</p>