[Mesa-dev] [PATCH] gallivm: make sampling more robust against bogus coordinates

Cherniak, Bruce bruce.cherniak at intel.com
Fri Apr 22 18:57:17 UTC 2016


Tested-by: Bruce Cherniak <bruce.cherniak at intel.com>





On 4/22/16, 8:33 AM, "mesa-dev on behalf of sroland at vmware.com" <mesa-dev-bounces at lists.freedesktop.org on behalf of 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:
>-- 
>2.1.4
>
>_______________________________________________
>mesa-dev mailing list
>mesa-dev at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list