From bd07e20d208268382a34dca23ff71a8192bb1525 Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Tue, 26 Apr 2016 04:53:01 +0200 Subject: [PATCH] gallivm: make sampling more robust against bogus coordinates 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 No piglit changes. (v2: fix min/max typo in coord_mirror, add comment) Cc: "11.1 11.2" Tested-by: Bruce Cherniak Reviewed-by: Jose Fonseca --- src/gallium/auxiliary/gallivm/lp_bld_arit.c | 9 ++--- .../auxiliary/gallivm/lp_bld_sample_aos.c | 13 ++++++- .../auxiliary/gallivm/lp_bld_sample_soa.c | 34 ++++++++++++++----- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c b/src/gallium/auxiliary/gallivm/lp_bld_arit.c index beff4143fd8..17cf296fc20 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 729c5b8f6ef..6bf92c87c49 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 332394e0f04..31cf74f0ceb 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c @@ -228,11 +228,16 @@ 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 */ + /* XXX: not safe without arch rounding, fract can be anything. */ + fract = lp_build_max_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 +277,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 +385,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 +409,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 +476,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 +640,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: -- 2.30.2