Francisco Jerez <curroje...@riseup.net> writes: > Jason Ekstrand <ja...@jlekstrand.net> writes: > >> We have had a guard against OOB array access of images on IVB for a long >> time, but it can actually cause hangs on any GPU generation. This can >> happen due to getting an untyped SURFACE_STATE for a typed message or >> vice-versa. > > Are you sure about that? AFAIK the > "arb_shader_image_load_store/invalid/format mismatch" subtest already > leads to a typed/untyped mismatch on HSW and I'm not aware of it causing > hangs -- Not saying that the fix below is wrong, the code looks > reasonable but I'm skeptical that the reason for the hang is simply a > surface format mismatch. > With our off-line discussion in mind, if you s/or vice-versa// above and s/push/pull/ below this patch is:
Reviewed-by: Francisco Jerez <curroje...@riseup.net> >> We didn't used to hit this with the piglit test on anything >> other than IVB because the OOB in the test would cause us to go past the >> top of the push constant UBO and we would get a surface index of 0 which is > > You probably meant the pull constant buffer? > >> was always a valid surface. Now that we're pushing small arrays, we can >> end up grabbing garbage from the GRF and going to some random index which >> causes a hang. The solution is to just do the bounds check on all >> hardware. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94944 >> --- >> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 26 +++++++++++--------------- >> 1 file changed, 11 insertions(+), 15 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> index c16f1ed..e775049 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> @@ -1272,21 +1272,17 @@ fs_visitor::get_nir_image_deref(const nir_deref_var >> *deref) >> if (deref_array->deref_array_type == nir_deref_array_type_indirect) { >> fs_reg tmp = vgrf(glsl_type::uint_type); >> >> - if (devinfo->gen == 7 && !devinfo->is_haswell) { >> - /* IVB hangs when trying to access an invalid surface index with >> - * the dataport. According to the spec "if the index used to >> - * select an individual element is negative or greater than or >> - * equal to the size of the array, the results of the operation >> - * are undefined but may not lead to termination" -- which is >> one >> - * of the possible outcomes of the hang. Clamp the index to >> - * prevent access outside of the array bounds. >> - */ >> - bld.emit_minmax(tmp, retype(get_nir_src(deref_array->indirect), >> - BRW_REGISTER_TYPE_UD), >> - brw_imm_ud(size - base - 1), BRW_CONDITIONAL_L); >> - } else { >> - bld.MOV(tmp, get_nir_src(deref_array->indirect)); >> - } >> + /* Accessing an invalid surface index with the dataport can result >> + * in a hang. According to the spec "if the index used to >> + * select an individual element is negative or greater than or >> + * equal to the size of the array, the results of the operation >> + * are undefined but may not lead to termination" -- which is one >> + * of the possible outcomes of the hang. Clamp the index to >> + * prevent access outside of the array bounds. >> + */ >> + bld.emit_minmax(tmp, retype(get_nir_src(deref_array->indirect), >> + BRW_REGISTER_TYPE_UD), >> + brw_imm_ud(size - base - 1), BRW_CONDITIONAL_L); >> >> indirect_max += element_size * (tail->type->length - 1); >> >> -- >> 2.5.0.400.gff86faf >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
