[Libclc] Resolve FIXME: GCN insel crashes when a == 0 or b == 0

[arsenm]

I don't understand what you are trying to fix. What crashes exactly?

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[jvesely]

In D107849#2939767, @gAlfonso-bit wrote:

In D107849#2937885, @arsenm wrote:

I don't understand what you are trying to fix. What crashes exactly?

It doesn’t crash anymore, so we can remove the workarounds in place. That’s the point.

hi,

sorry for the long delay. Which LLVM versions have you tested?

Jan

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

libclc/generic/lib/math/clc_fma.cl

 };
 
 _CLC_DEF _CLC_OVERLOAD float __clc_sw_fma(float a, float b, float c)
 {
   /* special cases */
   if (isnan(a) || isnan(b) || isnan(c) || isinf(a) || isinf(b))
     return mad(a, b, c);
 
-  /* If only c is inf, and both a,b are regular numbers, the result is c*/
-  if (isinf(c))
+  /* If only c is inf, and both a,b are regular numbers, or if a or b is 0, the result is c*/
+  if (a == .0f || b == .0f || isinf(c))
     return c;
 
   a = __clc_flush_denormal_if_not_supported(a);
   b = __clc_flush_denormal_if_not_supported(b);
   c = __clc_flush_denormal_if_not_supported(c);
 
-  if (c == 0)
+  if (c == .0f)
     return a * b;
 
   struct fp st_a, st_b, st_c;
 
-  st_a.exponent = a == .0f ? 0 : ((as_uint(a) & 0x7f800000) >> 23) - 127;
-  st_b.exponent = b == .0f ? 0 : ((as_uint(b) & 0x7f800000) >> 23) - 127;
-  st_c.exponent = c == .0f ? 0 : ((as_uint(c) & 0x7f800000) >> 23) - 127;
-
-  st_a.mantissa = a == .0f ? 0 : (as_uint(a) & 0x7fffff) | 0x800000;
-  st_b.mantissa = b == .0f ? 0 : (as_uint(b) & 0x7fffff) | 0x800000;
-  st_c.mantissa = c == .0f ? 0 : (as_uint(c) & 0x7fffff) | 0x800000;
+  st_a.exponent = ((as_uint(a) & 0x7f800000) >> 23) - 127;
+  st_b.exponent = ((as_uint(b) & 0x7f800000) >> 23) - 127;
+
+  st_a.mantissa = (as_uint(a) & 0x7fffff) | 0x800000;
+  st_b.mantissa = (as_uint(b) & 0x7fffff) | 0x800000;
 
   st_a.sign = as_uint(a) & 0x80000000;
   st_b.sign = as_uint(b) & 0x80000000;
-  st_c.sign = as_uint(c) & 0x80000000;
 
   // Multiplication.
   // Move the product to the highest bits to maximize precision
   // mantissa is 24 bits => product is 48 bits, 2bits non-fraction.
   // Add one bit for future addition overflow,
   // add another bit to detect subtraction underflow
   struct fp st_mul;
   st_mul.sign = st_a.sign ^ st_b.sign;
   st_mul.mantissa = (st_a.mantissa * st_b.mantissa) << 14ul;
-  st_mul.exponent = st_mul.mantissa ? st_a.exponent + st_b.exponent : 0;
-
-  // FIXME: Detecting a == 0 || b == 0 above crashed GCN isel
-  if (st_mul.exponent == 0 && st_mul.mantissa == 0)
-    return c;
+  st_mul.exponent = st_a.exponent + st_b.exponent;
 
 // Mantissa is 23 fractional bits, shift it the same way as product mantissa
 #define C_ADJUST 37ul
 
+  st_c.exponent = ((as_uint(c) & 0x7f800000) >> 23) - 127;
+  st_c.mantissa = ((as_uint(c) & 0x7fffff) | 0x800000) << C_ADJUST;
+  st_c.sign = as_uint(c) & 0x80000000;
+
+  struct fp *greater;
+  struct fp *less;
   // both exponents are bias adjusted
-  int exp_diff = st_mul.exponent - st_c.exponent;
+  if (st_mul.exponent > st_c.exponent) {
+    greater = &st_mul;
+    less = &st_c;
+  }
+  else
+  {
+    greater = &st_c;
+    less = &st_mul;
+  }
+
+  uint exp_diff = greater->exponent - less->exponent;
+
+  const ulong cutoff_mask = (1ul << exp_diff) - 1ul;
+  ulong cutoff_bits;
 
-  st_c.mantissa <<= C_ADJUST;
-  ulong cutoff_bits = 0;
-  ulong cutoff_mask = (1ul << abs(exp_diff)) - 1ul;
-  if (exp_diff > 0) {
-    cutoff_bits = exp_diff >= 64 ? st_c.mantissa : (st_c.mantissa & cutoff_mask);
-    st_c.mantissa = exp_diff >= 64 ? 0 : (st_c.mantissa >> exp_diff);
+  if (exp_diff < 64) {
+    cutoff_bits = (less->mantissa & cutoff_mask);
+    less->mantissa = (less->mantissa >> exp_diff)
   } else {
-    cutoff_bits = -exp_diff >= 64 ? st_mul.mantissa : (st_mul.mantissa & cutoff_mask);
-    st_mul.mantissa = -exp_diff >= 64 ? 0 : (st_mul.mantissa >> -exp_diff);
-  }
+    cutoff_bits = less->mantissa;
+    less->mantissa = 0;  
+  } 
 
   struct fp st_fma;
   st_fma.sign = st_mul.sign;
-  st_fma.exponent = max(st_mul.exponent, st_c.exponent);
+  st_fma.exponent = greater->exponent;
   if (st_c.sign == st_mul.sign) {
     st_fma.mantissa = st_mul.mantissa + st_c.mantissa;
   } else {
     // cutoff bits borrow one
     st_fma.mantissa = st_mul.mantissa - st_c.mantissa - (cutoff_bits && (st_mul.exponent > st_c.exponent) ? 1 : 0);
   }
 
   // underflow: st_c.sign != st_mul.sign, and magnitude switches the sign
#define C_ADJUST 37ul
   // Flating point range limit
   if (st_fma.exponent > 127)
     return as_float(as_uint(INFINITY) | st_fma.sign);
 
   // Flush denormals
   if (st_fma.exponent <= -127)
     return as_float(st_fma.sign);
 
-  return as_float(st_fma.sign | ((st_fma.exponent + 127) << 23) | ((uint)st_fma.mantissa & 0x7fffff));
+  return as_float(st_fma.sign | ((st_fma.exponent + 127) << 23) | (st_fma.mantissa & 0x7fffff));
 }
 _CLC_TERNARY_VECTORIZE(_CLC_DEF _CLC_OVERLOAD, float, __clc_sw_fma, float, float, float)

libclc/generic/lib/math/math.h

 #define BIASEDEMIN_SP32   1
 #define EMIN_SP32         -126
 #define BIASEDEMAX_SP32   254
 #define EMAX_SP32         127
 #define LAMBDA_SP32       1.0e30
 #define MANTLENGTH_SP32   24
 #define BASEDIGITS_SP32   7
 
-_CLC_OVERLOAD _CLC_INLINE float __clc_flush_denormal_if_not_supported(float x)
-{
-  int ix = as_int(x);
-  if (!__clc_fp32_subnormals_supported() &&
-    ((ix & EXPBITS_SP32) == 0) && ((ix & MANTBITS_SP32) != 0)) {
-    ix &= SIGNBIT_SP32;
-    x = as_float(ix);
-  }
-  return x;
+_CLC_OVERLOAD _CLC_INLINE float __clc_flush_denormal_if_not_supported(float x) {
+  int ix = as_int(x);
+  if (!__clc_fp32_subnormals_supported() && ((ix & MANTBITS_SP32) != 0) &&
+      ((ix & EXPBITS_SP32) == 0)) {
+    ix &= SIGNBIT_SP32;
+    x = as_float(ix);
+  }
+  return x;
 }
 
 #ifdef cl_khr_fp64
 
 #define SIGNBIT_DP64      0x8000000000000000L
 #define EXSIGNBIT_DP64    0x7fffffffffffffffL
 #define EXPBITS_DP64      0x7ff0000000000000L
 #define MANTBITS_DP64     0x000fffffffffffffL