Expand _01 half<->float limitation to _finite. Simplify.

src/core/SkHalf.h

Index: src/core/SkHalf.h
diff --git a/src/core/SkHalf.h b/src/core/SkHalf.h
index 5f5575ae1aeaede7f1fd6bde72c8df549f8557bf..582fb21ed15985aa0cc1c1987b1c98487a38f279 100644
--- a/src/core/SkHalf.h
+++ b/src/core/SkHalf.h
@@ -24,10 +24,10 @@ typedef uint16_t SkHalf;
 float SkHalfToFloat(SkHalf h);
 SkHalf SkFloatToHalf(float f);
 
-// Convert between half and single precision floating point, but pull any dirty
-// trick we can to make it faster as long as it's correct enough for values in [0,1].
-static inline     Sk4f SkHalfToFloat_01(uint64_t);
-static inline uint64_t SkFloatToHalf_01(const Sk4f&);
+// Convert between half and single precision floating point,
+// assuming inputs and outputs are both finite.
+static inline     Sk4f SkHalfToFloat_finite(uint64_t);
+static inline uint64_t SkFloatToHalf_finite(const Sk4f&);
 
 // ~~~~~~~~~~~ impl ~~~~~~~~~~~~~~ //
 
@@ -36,7 +36,7 @@ static inline uint64_t SkFloatToHalf_01(const Sk4f&);
 
 // GCC 4.9 lacks the intrinsics to use ARMv8 f16<->f32 instructions, so we use inline assembly.
 
-static inline Sk4f SkHalfToFloat_01(uint64_t hs) {
+static inline Sk4f SkHalfToFloat_finite(uint64_t hs) {
 #if !defined(SKNX_NO_SIMD) && defined(SK_CPU_ARM64)
     float32x4_t fs;
     asm ("fmov  %d[fs], %[hs]        \n"   // vcreate_f16(hs)
@@ -44,53 +44,29 @@ static inline Sk4f SkHalfToFloat_01(uint64_t hs) {
         : [fs] "=w" (fs)                   // =w: write-only NEON register
         : [hs] "r" (hs));                  //  r: read-only 64-bit general register
     return fs;
-
-#elif !defined(SKNX_NO_SIMD) && defined(SK_ARM_HAS_NEON)
-    // NEON makes this pretty easy:
-    //   - denormals are 10-bit * 2^-14 == 24-bit fixed point;
-    //   - handle normals the same way as in SSE: align mantissa, then rebias exponent.
-    uint32x4_t h = vmovl_u16(vcreate_u16(hs)),
-               is_denorm = vcltq_u32(h, vdupq_n_u32(1<<10));
-    float32x4_t denorm = vcvtq_n_f32_u32(h, 24),
-                  norm = vreinterpretq_f32_u32(vaddq_u32(vshlq_n_u32(h, 13),
-                                                         vdupq_n_u32((127-15) << 23)));
-    return vbslq_f32(is_denorm, denorm, norm);
-
-#elif !defined(SKNX_NO_SIMD) && SK_CPU_SSE_LEVEL >= SK_CPU_SSE_LEVEL_SSE2
-    // If our input is a normal 16-bit float, things are pretty easy:
-    //   - shift left by 13 to put the mantissa in the right place;
-    //   - the exponent is wrong, but it just needs to be rebiased;
-    //   - re-bias the exponent from 15-bias to 127-bias by adding (127-15).
-
-    // If our input is denormalized, we're going to do the same steps, plus a few more fix ups:
-    //   - the input is h = K*2^-14, for some 10-bit fixed point K in [0,1);
-    //   - by shifting left 13 and adding (127-15) to the exponent, we constructed the float value
-    //     2^-15*(1+K);
-    //   - we'd need to subtract 2^-15 and multiply by 2 to get back to K*2^-14, or equivallently
-    //     multiply by 2 then subtract 2^-14.
-    //
-    //   - We'll work that multiply by 2 into the rebias, by adding 1 more to the exponent.
-    //   - Conveniently, this leaves that rebias constant 2^-14, exactly what we want to subtract.
-
-    __m128i h = _mm_unpacklo_epi16(_mm_loadl_epi64((const __m128i*)&hs), _mm_setzero_si128());
-    const __m128i is_denorm = _mm_cmplt_epi32(h, _mm_set1_epi32(1<<10));
-
-    __m128i rebias = _mm_set1_epi32((127-15) << 23);
-    rebias = _mm_add_epi32(rebias, _mm_and_si128(is_denorm, _mm_set1_epi32(1<<23)));
-
-    __m128i f = _mm_add_epi32(_mm_slli_epi32(h, 13), rebias);
-    return _mm_sub_ps(_mm_castsi128_ps(f),
-                      _mm_castsi128_ps(_mm_and_si128(is_denorm, rebias)));
 #else
-    float fs[4];
-    for (int i = 0; i < 4; i++) {
-        fs[i] = SkHalfToFloat(hs >> (i*16));
-    }
-    return Sk4f::Load(fs);
+    // Expand the halfs up to 32 bits each, and strip off the sign bit.
+    Sk4i positive = SkNx_cast<int>(Sk4h::Load(&hs)),

[msarett, 2016/07/13 22:07:05]

nit:

I found this code block confusing because "positive" is defined before the sign
bit is stripped.

This would be more clear:
Sk4i positive = SkNx_cast<int>(Sk4h::Load(&hs)) & 0x00007FFF;

But I guess since you use "sign" down below this doesn't save anything (and uses
another constant).

+         sign     = positive & 0x00008000;
+    positive ^= sign;
+
+    // For normal half floats, align the exponent/mantissa line and rebias the exponent.

[msarett, 2016/07/13 22:07:05]

This is the simplest part, and still, this code is really complicated!  Can we
try to make it clearer?

Ex:

static constexpr int kF32MantissaBits = 23;
static constexpr int kF32Bias = 127;
static constexpr int kF16MantissaBits = 10;
static constexpr int kF16Bias = 15;

// Align the f16 exponent/mantissa line with the f32 exponent/mantissa line
x = x << (kF32MantissaBits - kF16MantissaBits);

// Rebias the exponent
x = x + ((kF32Bias - kF16Bias) << kF32MantissaBits)

+    Sk4i norm = (positive << 13) + (112<<23);
+
+    // For denorm half floats, mask in a value with the right exponent for 2^-14,

[msarett, 2016/07/13 22:07:05]

I think the comment that would have made things much clearer for me is about why
it's ok to put the exponent bits of the float16 in the mantissa of the float32.

[msarett, 2016/07/14 12:39:23]

Oh duh the exponent bits are all zero.

+    // then subtract it off as a float.  This leaves just our original fraction.

[msarett, 2016/07/14 12:59:50]

// Desired exponent is 2^-14 because that is the exponent on denormal half
floats (-bias + 1)

+    const Sk4i denorm_fixup = 126<<23;

[msarett, 2016/07/14 12:59:50]

// Because the bias is 127 this is an exponent of 2^-1.  But the mantissa is
also shifted right by 13, so we really have 2^-14.

+    Sk4i denorm = positive | denorm_fixup;

[msarett, 2016/07/13 22:07:05]

nit:

Confused by the variable name.  It's not "denorm" here.  It's just a bunch of
bits.

I guess this is the same style pattern as above.  It's not my personal
preference, but I don't feel too strongly.

+    Sk4f denorm_f = Sk4f::Load(&denorm) - Sk4f::Load(&denorm_fixup);

[msarett, 2016/07/14 12:59:50]

// ((1 * 2^-1) + value) - (1 * 2^-1) = value

+    denorm = Sk4i::Load(&denorm_f);
+
+    Sk4i is_denorm = positive < (1<<10);  // Exponent == 0?
+    Sk4i merged = (sign << 16) | is_denorm.thenElse(denorm, norm);
+    return Sk4f::Load(&merged);
 #endif
 }
 
-static inline uint64_t SkFloatToHalf_01(const Sk4f& fs) {
+static inline uint64_t SkFloatToHalf_finite(const Sk4f& fs) {
     uint64_t r;
 #if !defined(SKNX_NO_SIMD) && defined(SK_CPU_ARM64)
     float32x4_t vec = fs.fVec;
@@ -98,25 +74,23 @@ static inline uint64_t SkFloatToHalf_01(const Sk4f& fs) {
          "fmov  %[r], %d[vec]         \n"   // vst1_f16(&r, ...)
         : [r] "=r" (r)                      // =r: write-only 64-bit general register
         , [vec] "+w" (vec));                // +w: read-write NEON register
-
-// TODO: ARMv7 NEON float->half?
-
-#elif !defined(SKNX_NO_SIMD) && SK_CPU_SSE_LEVEL >= SK_CPU_SSE_LEVEL_SSE2
-    // Scale down from 127-bias to 15-bias, then cut off bottom 13 mantissa bits.
-    // This doesn't round, so it can be 1 bit too small.
-    const __m128 rebias = _mm_castsi128_ps(_mm_set1_epi32((127 - (127-15)) << 23));
-    __m128i h = _mm_srli_epi32(_mm_castps_si128(_mm_mul_ps(fs.fVec, rebias)), 13);
-    _mm_storel_epi64((__m128i*)&r, _mm_packs_epi32(h,h));
-
 #else
-    SkHalf hs[4];
-    for (int i = 0; i < 4; i++) {
-        hs[i] = SkFloatToHalf(fs[i]);
-    }
-    r = (uint64_t)hs[3] << 48
-      | (uint64_t)hs[2] << 32
-      | (uint64_t)hs[1] << 16
-      | (uint64_t)hs[0] <<  0;
+    // Strip the sign bit from each float.
+    Sk4i positive = Sk4i::Load(&fs),
+         sign     = positive & 0x80000000;
+    positive ^= sign;
+
+    // Whether we'll produce normal or denorm half float results, either
+    // way we just invert the logic from SkHalfToFloat_finite() above.
+    Sk4i norm = (positive - (112<<23)) >> 13;

[msarett, 2016/07/14 12:59:50]

nit:
Still think this is clearer with constants.

// What happens when the exponent is less than 112?  It'll be a denormal
half-float so it doesn't matter anyway?

+
+    const Sk4i denorm_fixup = 126<<23;

[msarett, 2016/07/13 22:07:05]

Haven't looked here yet...

+    Sk4f denorm_f = Sk4f::Load(&positive) + Sk4f::Load(&denorm_fixup);

[msarett, 2016/07/14 12:59:50]

// (1 * 2^-1) + small float effectively shifts the small float down to the
bottom ten bits of the mantissa.

+    Sk4i denorm = Sk4i::Load(&denorm_f) ^ denorm_fixup;

[msarett, 2016/07/14 12:59:50]

Cool this saves us a mask.

// Mask away the exponent bits.

+
+    Sk4i will_be_denorm = positive < ((127-14) << 23);
+    Sk4i merged = (sign >> 16) | will_be_denorm.thenElse(denorm, norm);
+    SkNx_cast<uint16_t>(merged).store(&r);
 #endif
     return r;
 }