[Mesa-dev] [PATCH] gallivm: make sampling more robust against bogus coordinates
sroland at vmware.com
sroland at vmware.com
Fri Apr 22 13:33:57 UTC 2016
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:
--
2.1.4
More information about the mesa-dev
mailing list