nir: Fix i64tof32 lowering
authorBoris Brezillon <boris.brezillon@collabora.com>
Wed, 12 Aug 2020 12:28:48 +0000 (14:28 +0200)
committerMarge Bot <eric+marge@anholt.net>
Mon, 17 Aug 2020 20:31:31 +0000 (20:31 +0000)
The round-to-nearest-even implementation found in lower_2f() is incorrect
for any value having a significand that is not directly representable
and whose non-representable part lies between 1 and half the minimum
representable value. In this case, the significand is rounded up instead
of being rounded down.

Fixes: 936c58c8fcce ("nir: Extend nir_lower_int64() to support i2f/f2i lowering")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Tested-by: Jesse Natalie <jenatali@microsoft.com>
Acked-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6290>

src/compiler/nir/nir_lower_int64.c

index 03d7c1568071ea9053a55691ea1ba05199689878..9d268a843633283864f2e7019915eb96e9c0c59f 100644 (file)
@@ -714,35 +714,30 @@ lower_2f(nir_builder *b, nir_ssa_def *x, unsigned dest_bit_size,
       unreachable("Invalid dest_bit_size");
    }
 
-   /* We keep one more bit than can fit in the significand field to let the
-    * u2f32 conversion do the rounding for us.
-    */
    nir_ssa_def *discard =
-      nir_imax(b, nir_isub(b, exp, nir_imm_int(b, significand_bits + 1)),
+      nir_imax(b, nir_isub(b, exp, nir_imm_int(b, significand_bits)),
                   nir_imm_int(b, 0));
-
-   /* Part of the "round to nearest" has to be taken care of before we discard
-    * the LSB, and that's what this extra iadd is for.
-    * "Round to nearest even" is handled by u2f. That works because the
-    * shifted value either fits in the significand field (which means no
-    * rounding is required) or contains one extra bit that forces the
-    * conversion op to round things properly.
-    */
-   nir_ssa_def *add = COND_LOWER_OP(b, ishl, nir_imm_int64(b, 1), discard);
-   add = COND_LOWER_OP(b, isub, add, nir_imm_int64(b, 1));
-   nir_ssa_def *rounded_x = COND_LOWER_OP(b, iadd, x, add);
-
-   /* Signed Values can't overflow because we've saved the sign and promoted
-    * them to unsigned values.
+   nir_ssa_def *significand =
+      COND_LOWER_CAST(b, u2u32, COND_LOWER_OP(b, ushr, x, discard));
+
+   /* Round-to-nearest-even implementation:
+    * - if the non-representable part of the significand is higher than half
+    *   the minimum representable significand, we round-up
+    * - if the non-representable part of the significand is equal to half the
+    *   minimum representable significand and the representable part of the
+    *   significand is odd, we round-up
+    * - in any other case, we round-down
     */
-   if (!src_is_signed) {
-      nir_ssa_def *overflow = COND_LOWER_CMP(b, ult, rounded_x, x);
-      rounded_x = COND_LOWER_OP(b, bcsel, overflow,
-                                nir_imm_int64(b, UINT64_MAX), rounded_x);
-   }
-
-   nir_ssa_def *significand = COND_LOWER_OP(b, ushr, rounded_x, discard);
-   significand = COND_LOWER_CAST(b, u2u32, significand);
+   nir_ssa_def *lsb_mask = COND_LOWER_OP(b, ishl, nir_imm_int64(b, 1), discard);
+   nir_ssa_def *rem_mask = COND_LOWER_OP(b, isub, lsb_mask, nir_imm_int64(b, 1));
+   nir_ssa_def *half = COND_LOWER_OP(b, ishr, lsb_mask, nir_imm_int(b, 1));
+   nir_ssa_def *rem = COND_LOWER_OP(b, iand, x, rem_mask);
+   nir_ssa_def *halfway = nir_iand(b, COND_LOWER_CMP(b, ieq, rem, half),
+                                   nir_ine(b, discard, nir_imm_int(b, 0)));
+   nir_ssa_def *is_odd = nir_i2b(b, nir_iand(b, significand, nir_imm_int(b, 1)));
+   nir_ssa_def *round_up = nir_ior(b, COND_LOWER_CMP(b, ilt, half, rem),
+                                   nir_iand(b, halfway, is_odd));
+   significand = nir_iadd(b, significand, nir_b2i32(b, round_up));
 
    nir_ssa_def *res;