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" <mesa-stable@lists.freedesktop.org>
Tested-by: Bruce Cherniak <bruce.cherniak@intel.com>
Reviewed-by: Jose Fonseca <jfonseca@vmware.com>

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index beff4143fd8cc17ae3b53ccc88835c30dc203f15..17cf296fc20faa3fd996525e8d73c968341f877a 100644 (file)
--- 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 729c5b8f6ef21fd8f2fd6ffb6c590c3667a812a8..6bf92c87c49994262b43ec3282f24cfb107cf67e 100644 (file)
--- 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 332394e0f0465e9ff1f8580e4fc0f0a14a27d3d9..31cf74f0ceb5b50b1e01d31af99a4ce86167db80 100644 (file)
--- 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: