diff mbox

Fix PR63914

Message ID alpine.LSU.2.11.1411181352080.374@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Nov. 18, 2014, 12:55 p.m. UTC
The following fixes an ICE in CCPs set_lattice_value which is
confused when transitioning from { NaN, 0.0 } to { 0.0, 0.0 }.
The comment before canonicalize_value nicely explains what
happens here (with -ffinite-math-only) but the function fails
to handle float vectors or complex float constants.  Now -- we
cannot simply canonicalize { NaN, 0.0 } to UNDEFINED as the
value is still partly defined.  Thus instead of doing the
other possible thing (drop to VARYING here) I drop the
canonicalization we do for the !HONOR_NANS case (as well
as the !HONOR_SIGNED_ZEROS as operand_equal_p deals nicely
with that).  Instead of canonicalizing I allow this kind of
lattice transitions (from NaN to !NaN if !HONOR_NANS).

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
sofar.

I've chosen to change the assert to a gcc_checking_assert which
should be safe to backport and would hide the bug.

Richard.

2014-11-18  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/63914
	* tree-ssa-ccp.c (canonicalize_value): Remove float value
	canonicalization.
	(valid_lattice_transition): Allow (partial) transition
	from NaN to non-NaN if !HONOR_NANS.
	(set_lattice_value): Check for valid lattice transitions
	only when checking is enabled.

	* gcc.dg/pr63914.c: New testcase.
diff mbox

Patch

Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c	(revision 217653)
+++ gcc/tree-ssa-ccp.c	(working copy)
@@ -402,58 +402,16 @@  set_value_varying (tree var)
   val->mask = -1;
 }
 
-/* For float types, modify the value of VAL to make ccp work correctly
-   for non-standard values (-0, NaN):
-
-   If HONOR_SIGNED_ZEROS is false, and VAL = -0, we canonicalize it to 0.
-   If HONOR_NANS is false, and VAL is NaN, we canonicalize it to UNDEFINED.
-     This is to fix the following problem (see PR 29921): Suppose we have
-
-     x = 0.0 * y
-
-     and we set value of y to NaN.  This causes value of x to be set to NaN.
-     When we later determine that y is in fact VARYING, fold uses the fact
-     that HONOR_NANS is false, and we try to change the value of x to 0,
-     causing an ICE.  With HONOR_NANS being false, the real appearance of
-     NaN would cause undefined behavior, though, so claiming that y (and x)
-     are UNDEFINED initially is correct.
-
-  For other constants, make sure to drop TREE_OVERFLOW.  */
+/* For integer constants, make sure to drop TREE_OVERFLOW.  */
 
 static void
 canonicalize_value (ccp_prop_value_t *val)
 {
-  machine_mode mode;
-  tree type;
-  REAL_VALUE_TYPE d;
-
   if (val->lattice_val != CONSTANT)
     return;
 
   if (TREE_OVERFLOW_P (val->value))
     val->value = drop_tree_overflow (val->value);
-
-  if (TREE_CODE (val->value) != REAL_CST)
-    return;
-
-  d = TREE_REAL_CST (val->value);
-  type = TREE_TYPE (val->value);
-  mode = TYPE_MODE (type);
-
-  if (!HONOR_SIGNED_ZEROS (mode)
-      && REAL_VALUE_MINUS_ZERO (d))
-    {
-      val->value = build_real (type, dconst0);
-      return;
-    }
-
-  if (!HONOR_NANS (mode)
-      && REAL_VALUE_ISNAN (d))
-    {
-      val->lattice_val = UNDEFINED;
-      val->value = NULL;
-      return;
-    }
 }
 
 /* Return whether the lattice transition is valid.  */
@@ -487,7 +445,49 @@  valid_lattice_transition (ccp_prop_value
 	    == wi::bit_and_not (wi::to_widest (new_val.value), new_val.mask));
 
   /* Otherwise constant values have to agree.  */
-  return operand_equal_p (old_val.value, new_val.value, 0);
+  if (operand_equal_p (old_val.value, new_val.value, 0))
+    return true;
+
+  /* At least the kinds and types should agree now.  */
+  if (TREE_CODE (old_val.value) != TREE_CODE (new_val.value)
+      || !types_compatible_p (TREE_TYPE (old_val.value),
+			      TREE_TYPE (new_val.value)))
+    return false;
+
+  /* For floats and !HONOR_NANS allow transitions from (partial) NaN
+     to non-NaN.  */
+  tree type = TREE_TYPE (new_val.value);
+  if (SCALAR_FLOAT_TYPE_P (type)
+      && !HONOR_NANS (TYPE_MODE (type)))
+    {
+      if (REAL_VALUE_ISNAN (TREE_REAL_CST (old_val.value)))
+	return true;
+    }
+  else if (VECTOR_FLOAT_TYPE_P (type)
+	   && !HONOR_NANS (TYPE_MODE (TREE_TYPE (type))))
+    {
+      for (unsigned i = 0; i < VECTOR_CST_NELTS (old_val.value); ++i)
+	if (!REAL_VALUE_ISNAN
+	       (TREE_REAL_CST (VECTOR_CST_ELT (old_val.value, i)))
+	    && !operand_equal_p (VECTOR_CST_ELT (old_val.value, i),
+				 VECTOR_CST_ELT (new_val.value, i), 0))
+	  return false;
+      return true;
+    }
+  else if (COMPLEX_FLOAT_TYPE_P (type)
+	   && !HONOR_NANS (TYPE_MODE (TREE_TYPE (type))))
+    {
+      if (!REAL_VALUE_ISNAN (TREE_REAL_CST (TREE_REALPART (old_val.value)))
+	  && !operand_equal_p (TREE_REALPART (old_val.value),
+			       TREE_REALPART (new_val.value), 0))
+	return false;
+      if (!REAL_VALUE_ISNAN (TREE_REAL_CST (TREE_IMAGPART (old_val.value)))
+	  && !operand_equal_p (TREE_IMAGPART (old_val.value),
+			       TREE_IMAGPART (new_val.value), 0))
+	return false;
+      return true;
+    }
+  return false;
 }
 
 /* Set the value for variable VAR to NEW_VAL.  Return true if the new
@@ -514,7 +514,7 @@  set_lattice_value (tree var, ccp_prop_va
       new_val.mask = new_val.mask | old_val->mask | diff;
     }
 
-  gcc_assert (valid_lattice_transition (*old_val, new_val));
+  gcc_checking_assert (valid_lattice_transition (*old_val, new_val));
 
   /* If *OLD_VAL and NEW_VAL are the same, return false to inform the
      caller that this was a non-transition.  */
Index: gcc/testsuite/gcc.dg/pr63914.c
===================================================================
--- gcc/testsuite/gcc.dg/pr63914.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr63914.c	(working copy)
@@ -0,0 +1,45 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -ffast-math" } */
+
+typedef float __m128 __attribute__ ((__vector_size__ (16)));
+__m128 a, d, e;
+int b;
+struct dt_interpolation c;
+__m128
+fn1 (float p1)
+{
+  return (__attribute__ ((__vector_size__ (4 * sizeof 0))) float){ p1 };
+}
+__m128
+fn2 (float p1)
+{
+  return fn1 (p1);
+}
+struct dt_interpolation
+{
+  int width;
+};
+void
+fn3 (struct dt_interpolation *p1, int *p2)
+{
+  int i = 0, n = 0;
+  while (i < 2 * p1->width)
+    n = i++;
+  *p2 = n;
+}
+void
+fn4 ()
+{
+  __m128 f;
+  fn3 (&c, &b);
+  __m128 g = fn2 (1.f / b);
+  e = (__m128){};
+  __m128 h = e;
+  for (int i = 0; i < 2 * c.width; i++)
+    {
+      for (; c.width;)
+	f = a;
+      h = f;
+    }
+  d = h * g;
+}