[Mesa-dev] [PATCH] gallivm: make sampling more robust against bogus coordinates
Jose Fonseca
jfonseca at vmware.com
Mon Apr 25 13:44:27 UTC 2016
Looks good AFAICT. Can there be an impact in performance?
Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
I also think we should have piglit testcases for these things. Would it
be easy to modify one of the existing textrwap to use Nans? We need to
use BGRA8 and something else (e.g., RGBA32F) to covert both AoS/SoA paths.
Jose
On 22/04/16 14:33, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> Some cases (especially these using fract for coord wrapping) did not handle
> NaNs (or Infs) correctly - the following code assumed the fract result
> could not be outside [0,1], but if the input is a NaN (or +-Inf) the fract
> result was NaN - which then could produce out-of-bound offsets.
>
> (Note that the explicit NaN behavior changes for min/max on x86 sse don't
> result in actual changes in the generated jit code, but may on other
> architectures. Found by looking through all the wrap functions.)
>
> This fixes https://bugs.freedesktop.org/show_bug.cgi?id=94955
>
> Cc: "11.1 11.2" <mesa-stable at lists.freedesktop.org>
> ---
> src/gallium/auxiliary/gallivm/lp_bld_arit.c | 9 ++++---
> src/gallium/auxiliary/gallivm/lp_bld_sample_aos.c | 13 ++++++++-
> src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c | 33 +++++++++++++++++------
> 3 files changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> index beff414..17cf296 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> @@ -2069,8 +2069,8 @@ lp_build_fract(struct lp_build_context *bld,
>
>
> /**
> - * Prevent returning a fractional part of 1.0 for very small negative values of
> - * 'a' by clamping against 0.99999(9).
> + * Prevent returning 1.0 for very small negative values of 'a' by clamping
> + * against 0.99999(9). (Will also return that value for NaNs.)
> */
> static inline LLVMValueRef
> clamp_fract(struct lp_build_context *bld, LLVMValueRef fract)
> @@ -2080,13 +2080,14 @@ clamp_fract(struct lp_build_context *bld, LLVMValueRef fract)
> /* this is the largest number smaller than 1.0 representable as float */
> max = lp_build_const_vec(bld->gallivm, bld->type,
> 1.0 - 1.0/(1LL << (lp_mantissa(bld->type) + 1)));
> - return lp_build_min(bld, fract, max);
> + return lp_build_min_ext(bld, fract, max,
> + GALLIVM_NAN_RETURN_OTHER_SECOND_NONNAN);
> }
>
>
> /**
> * Same as lp_build_fract, but guarantees that the result is always smaller
> - * than one.
> + * than one. Will also return the smaller-than-one value for infs, NaNs.
> */
> LLVMValueRef
> lp_build_fract_safe(struct lp_build_context *bld,
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_aos.c b/src/gallium/auxiliary/gallivm/lp_bld_sample_aos.c
> index 729c5b8..6bf92c8 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_aos.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_aos.c
> @@ -246,6 +246,12 @@ lp_build_coord_repeat_npot_linear_int(struct lp_build_sample_context *bld,
> mask = lp_build_compare(int_coord_bld->gallivm, int_coord_bld->type,
> PIPE_FUNC_LESS, *coord0_i, int_coord_bld->zero);
> *coord0_i = lp_build_select(int_coord_bld, mask, length_minus_one, *coord0_i);
> + /*
> + * We should never get values too large - except if coord was nan or inf,
> + * in which case things go terribly wrong...
> + * Alternatively, could use fract_safe above...
> + */
> + *coord0_i = lp_build_min(int_coord_bld, *coord0_i, length_minus_one);
> }
>
>
> @@ -490,6 +496,10 @@ lp_build_sample_wrap_linear_float(struct lp_build_sample_context *bld,
> *coord1 = lp_build_add(coord_bld, coord, half);
> coord = lp_build_sub(coord_bld, coord, half);
> *weight = lp_build_fract(coord_bld, coord);
> + /*
> + * It is important for this comparison to be unordered
> + * (or need fract_safe above).
> + */
> mask = lp_build_compare(coord_bld->gallivm, coord_bld->type,
> PIPE_FUNC_LESS, coord, coord_bld->zero);
> *coord0 = lp_build_select(coord_bld, mask, length_minus_one, coord);
> @@ -514,7 +524,8 @@ lp_build_sample_wrap_linear_float(struct lp_build_sample_context *bld,
> coord = lp_build_sub(coord_bld, coord, half);
> }
> /* clamp to [0, length - 1] */
> - coord = lp_build_min(coord_bld, coord, length_minus_one);
> + coord = lp_build_min_ext(coord_bld, coord, length_minus_one,
> + GALLIVM_NAN_RETURN_OTHER_SECOND_NONNAN);
> coord = lp_build_max(coord_bld, coord, coord_bld->zero);
> *coord1 = lp_build_add(coord_bld, coord, coord_bld->one);
> /* convert to int, compute lerp weight */
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> index 1727105..ace24fd 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> @@ -228,11 +228,15 @@ lp_build_coord_mirror(struct lp_build_sample_context *bld,
> LLVMValueRef fract, flr, isOdd;
>
> lp_build_ifloor_fract(coord_bld, coord, &flr, &fract);
> + /* kill off NaNs */
> + fract = lp_build_min_ext(coord_bld, fract, coord_bld->zero,
> + GALLIVM_NAN_RETURN_OTHER_SECOND_NONNAN);
>
> /* isOdd = flr & 1 */
> isOdd = LLVMBuildAnd(bld->gallivm->builder, flr, int_coord_bld->one, "");
>
> /* make coord positive or negative depending on isOdd */
> + /* XXX slight overkill masking out sign bit is unnecessary */
> coord = lp_build_set_sign(coord_bld, fract, isOdd);
>
> /* convert isOdd to float */
> @@ -272,10 +276,15 @@ lp_build_coord_repeat_npot_linear(struct lp_build_sample_context *bld,
> * we avoided the 0.5/length division before the repeat wrap,
> * now need to fix up edge cases with selects
> */
> + /*
> + * Note we do a float (unordered) compare so we can eliminate NaNs.
> + * (Otherwise would need fract_safe above).
> + */
> + mask = lp_build_compare(coord_bld->gallivm, coord_bld->type,
> + PIPE_FUNC_LESS, coord_f, coord_bld->zero);
> +
> /* convert to int, compute lerp weight */
> lp_build_ifloor_fract(coord_bld, coord_f, coord0_i, weight_f);
> - mask = lp_build_compare(int_coord_bld->gallivm, int_coord_bld->type,
> - PIPE_FUNC_LESS, *coord0_i, int_coord_bld->zero);
> *coord0_i = lp_build_select(int_coord_bld, mask, length_minus_one, *coord0_i);
> }
>
> @@ -375,7 +384,8 @@ lp_build_sample_wrap_linear(struct lp_build_sample_context *bld,
> }
>
> /* clamp to length max */
> - coord = lp_build_min(coord_bld, coord, length_f);
> + coord = lp_build_min_ext(coord_bld, coord, length_f,
> + GALLIVM_NAN_RETURN_OTHER_SECOND_NONNAN);
> /* subtract 0.5 */
> coord = lp_build_sub(coord_bld, coord, half);
> /* clamp to [0, length - 0.5] */
> @@ -398,7 +408,7 @@ lp_build_sample_wrap_linear(struct lp_build_sample_context *bld,
> coord = lp_build_add(coord_bld, coord, offset);
> }
> /* was: clamp to [-0.5, length + 0.5], then sub 0.5 */
> - /* can skip clamp (though might not work for very large coord values */
> + /* can skip clamp (though might not work for very large coord values) */
> coord = lp_build_sub(coord_bld, coord, half);
> /* convert to int, compute lerp weight */
> lp_build_ifloor_fract(coord_bld, coord, &coord0, &weight);
> @@ -465,7 +475,8 @@ lp_build_sample_wrap_linear(struct lp_build_sample_context *bld,
> coord = lp_build_abs(coord_bld, coord);
>
> /* clamp to length max */
> - coord = lp_build_min(coord_bld, coord, length_f);
> + coord = lp_build_min_ext(coord_bld, coord, length_f,
> + GALLIVM_NAN_RETURN_OTHER_SECOND_NONNAN);
> /* subtract 0.5 */
> coord = lp_build_sub(coord_bld, coord, half);
> /* clamp to [0, length - 0.5] */
> @@ -628,9 +639,15 @@ lp_build_sample_wrap_nearest(struct lp_build_sample_context *bld,
>
> /* itrunc == ifloor here */
> icoord = lp_build_itrunc(coord_bld, coord);
> -
> - /* clamp to [0, length - 1] */
> - icoord = lp_build_min(int_coord_bld, icoord, length_minus_one);
> + /*
> + * Use unsigned min due to possible undef values (NaNs, overflow)
> + */
> + {
> + struct lp_build_context abs_coord_bld = *int_coord_bld;
> + abs_coord_bld.type.sign = FALSE;
> + /* clamp to [0, length - 1] */
> + icoord = lp_build_min(&abs_coord_bld, icoord, length_minus_one);
> + }
> break;
>
> case PIPE_TEX_WRAP_MIRROR_CLAMP_TO_BORDER:
>
More information about the mesa-dev
mailing list