Reviewed-by: Marek Olšák <marek.ol...@amd.com>
Marek
On Mon, Mar 21, 2016 at 9:41 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haeh...@amd.com>
>
> Results are undefined but may not crash. Without this change, out-of-bounds
> indexing can lead to VM faults and GPU hangs.
>
> Constant buffers, samplers, and possibly others will eventually need similar
> treatment to support GL_ARB_robust_buffer_access_behavior.
> ---
> src/gallium/drivers/radeonsi/si_shader.c | 44
> +++++++++++++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c
> b/src/gallium/drivers/radeonsi/si_shader.c
> index 9ad2290..1e4bf82 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -532,6 +532,37 @@ static LLVMValueRef get_indirect_index(struct
> si_shader_context *ctx,
> }
>
> /**
> + * Like get_indirect_index, but restricts the return value to a (possibly
> + * undefined) value inside [0..num).
> + */
> +static LLVMValueRef get_bounded_indirect_index(struct si_shader_context *ctx,
> + const struct tgsi_ind_register
> *ind,
> + int rel_index, unsigned num)
> +{
> + struct gallivm_state *gallivm = &ctx->radeon_bld.gallivm;
> + LLVMBuilderRef builder = gallivm->builder;
> + LLVMValueRef result = get_indirect_index(ctx, ind, rel_index);
> + LLVMValueRef c_max = LLVMConstInt(ctx->i32, num - 1, 0);
> + LLVMValueRef cc;
> +
> + if (util_is_power_of_two(num)) {
> + result = LLVMBuildAnd(builder, result, c_max, "");
> + } else {
> + /* In theory, this MAX pattern should result in code that is
> + * as good as the bit-wise AND above.
> + *
> + * In practice, LLVM generates worse code (at the time of
> + * writing), because its value tracking is not strong enough.
> + */
> + cc = LLVMBuildICmp(builder, LLVMIntULE, result, c_max, "");
> + result = LLVMBuildSelect(builder, cc, result, c_max, "");
> + }
> +
> + return result;
> +}
> +
> +
> +/**
> * Calculate a dword address given an input or output register and a stride.
> */
> static LLVMValueRef get_dw_address(struct si_shader_context *ctx,
> @@ -2814,7 +2845,18 @@ image_fetch_rsrc(
> LLVMValueRef rsrc_ptr;
> LLVMValueRef tmp;
>
> - ind_index = get_indirect_index(ctx, &image->Indirect,
> image->Register.Index);
> + /* From the GL_ARB_shader_image_load_store extension spec:
> + *
> + * If a shader performs an image load, store, or atomic
> + * operation using an image variable declared as an array,
> + * and 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.
> + */
> + ind_index = get_bounded_indirect_index(ctx, &image->Indirect,
> + image->Register.Index,
> + SI_NUM_IMAGES);
>
> rsrc_ptr = LLVMGetParam(ctx->radeon_bld.main_fn,
> SI_PARAM_IMAGES);
> tmp = build_indexed_load_const(ctx, rsrc_ptr, ind_index);
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
