On Tue, Sep 15, 2015 at 6:39 AM, Francisco Jerez <curroje...@riseup.net> wrote: > Jason Ekstrand <ja...@jlekstrand.net> writes: > >> In certain conditions, we have to do bounds-checking in the shader for >> image_load_store. The way this works for image loads is that we do the >> load anyway and then emit a series of slecects, one per component, that > > Strictly speaking the load is predicated so it's not really done for out > of bounds coordinates, with that sentence clarified or removed (and the > typo fixed in "slecects") this patch is:
Fixed. Sorry about that; I forgot that we predicated the loads. When I wrote that I thought it sounded odd. Out of curiosity, why don't we just emit a series of MOV dst.x 0 before the predicated load? It's the same number of instructions and might be able to be scheduled better. I guess doing the selects is more SSA-like. --Jason > Reviewed-by: Francisco Jerez <curroje...@riseup.net> > >> gives us 0 or the loaded value depending on whether or not you're in >> bounds. However, we were hard-coding 4 components which may not be >> correct. Instead, we should be using size which is the number of >> components read. >> >> Cc: Francisco Jerez <curroje...@riseup.net> > > Although this is unlikely to fix any preexisting bugs (because the > result of the extra SELs would have been left unused and they would have > been dead-code-eliminated later on anyway), it shouldn't hurt to CC this > to mesa-stable too, if you like. > >> --- >> src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp >> b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp >> index 727e8d1..88f22fa 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp >> @@ -905,7 +905,7 @@ namespace brw { >> tmp = emit_untyped_read(bld, image, laddr, 1, size, pred); >> >> /* An out of bounds surface access should give zero as result. >> */ >> - for (unsigned c = 0; c < 4; ++c) >> + for (unsigned c = 0; c < size; ++c) >> set_predicate(pred, bld.SEL(offset(tmp, bld, c), >> offset(tmp, bld, c), fs_reg(0))); >> } >> -- >> 2.5.0.400.gff86faf _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev