Tested-by: Bruce Cherniak <bruce.chern...@intel.com>
On 4/22/16, 8:33 AM, "mesa-dev on behalf of srol...@vmware.com" <mesa-dev-boun...@lists.freedesktop.org on behalf of srol...@vmware.com> wrote: >From: Roland Scheidegger <srol...@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-sta...@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: >-- >2.1.4 > >_______________________________________________ >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