[Mesa-dev] [PATCH] radeonsi: fix behavior of GLSL findLSB(0)

[Nicolai Hähnle]

On 26.10.2016 01:16, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> 12.0 and older need the same fix but elsewhere.
>
> Cc: 13.0 <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
> index 1ee9afb..123ff5d 100644
> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
> @@ -484,37 +484,46 @@ static void emit_bfi(const struct lp_build_tgsi_action *action,
>  				LLVMBuildXor(builder, bfi_args[1], bfi_args[2],
>  					     ""), ""), "");
>  }
>
>  /* this is ffs in C */
>  static void emit_lsb(const struct lp_build_tgsi_action *action,
>  		     struct lp_build_tgsi_context *bld_base,
>  		     struct lp_build_emit_data *emit_data)
>  {
>  	struct gallivm_state *gallivm = bld_base->base.gallivm;
> +	LLVMBuilderRef builder = gallivm->builder;
>  	LLVMValueRef args[2] = {
>  		emit_data->args[0],
>
>  		/* The value of 1 means that ffs(x=0) = undef, so LLVM won't
>  		 * add special code to check for x=0. The reason is that
>  		 * the LLVM behavior for x=0 is different from what we
> -		 * need here.
> -		 *
> -		 * The hardware already implements the correct behavior.
> +		 * need here. However, LLVM also assumes that ffs(x) is
> +		 * in [0, 31], but GLSL expects that ffs(0) = -1, so
> +		 * a conditional assignment to handle 0 is still required.
>  		 */
>  		LLVMConstInt(LLVMInt1TypeInContext(gallivm->context), 1, 0)
>  	};
>
> -	emit_data->output[emit_data->chan] =
> +	LLVMValueRef lsb =
>  		lp_build_intrinsic(gallivm->builder, "llvm.cttz.i32",
>  				emit_data->dst_type, args, ARRAY_SIZE(args),
>  				LLVMReadNoneAttribute);
> +
> +	/* TODO: We need an intrinsic to skip this conditional. */

An instruction selection pattern might do the trick as well. Anyway,

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

> +	/* Check for zero: */
> +	emit_data->output[emit_data->chan] =
> +		LLVMBuildSelect(builder,
> +				LLVMBuildICmp(builder, LLVMIntEQ, args[0],
> +					      bld_base->uint_bld.zero, ""),
> +				lp_build_const_int32(gallivm, -1), lsb, "");
>  }
>
>  /* Find the last bit set. */
>  static void emit_umsb(const struct lp_build_tgsi_action *action,
>  		      struct lp_build_tgsi_context *bld_base,
>  		      struct lp_build_emit_data *emit_data)
>  {
>  	struct gallivm_state *gallivm = bld_base->base.gallivm;
>  	LLVMBuilderRef builder = gallivm->builder;
>  	LLVMValueRef args[2] = {
>