Re: remove wrong code in immed_double_const

[Mike Stump <mikestump@comcast.net>]

On Mar 20, 2012, at 5:26 AM, Richard Sandiford wrote:
> So what I was trying to say was that if we remove the assert
> altogether, and allow CONST_DOUBLEs to be wider than 2 HWIs,
> we need to define what the "implicit" high-order HWIs of a
> CONST_DOUBLE are, just like we already do for CONST_INT.

Now, since you expressed a preference for sign extending, and a worry that there might be new bugs exposed in the handling of CONST_DOUBLEs in the face of my change, I went through all the code again and tried my best to fix every other bug in the compiler at all related to this area that I could find ; that patch is below.  In this one, I updated the spec for CONST_DOUBLE to be sign extending.

Curious, plus_constant is just terribly broken in this are, now fixed.  mode_signbit_p is speced in English, so, I didn't want to misread or misunderstand it and swizzle it, so I left it alone for now.   Someone will have to describe what it does and I can try my hand at fixing it, if broken, I suspect it is.  As for simplify_const_unary_operation, I don't know what they were thinking, return 0 seems safer to me.

If there is any other code that I missed that people know about, I'd be happy to fix it, just let me know what code.  I did a pass on all the ports as well, and they seem reasonably clean about it.  The biggest problem is OImode -1, would come out as hex digits, and all the upper 0xfffff digits implied by sign extension would be missing.  A port that cared about OImode, trivially, would fix their output routine.  The debugging code has similar problems.

Is this closer to something you think is in the right direction?  If so, let figure out the right solution for mode_signbit_p and proceed from there.


diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index de45a22..0c6dc45 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -1530,7 +1530,9 @@ Represents either a floating-point constant of mode @var{m} or an
 integer constant too large to fit into @code{HOST_BITS_PER_WIDE_INT}
 bits but small enough to fit within twice that number of bits (GCC
 does not provide a mechanism to represent even larger constants).  In
-the latter case, @var{m} will be @code{VOIDmode}.
+the latter case, @var{m} will be @code{VOIDmode}.  For integral values
+the value is a signed value, meaning the top bit of
+@code{CONST_DOUBLE_HIGH} is a sign bit.
 
 @findex CONST_DOUBLE_LOW
 If @var{m} is @code{VOIDmode}, the bits of the value are stored in
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 78ddfc3..c0b24e4 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -531,10 +531,9 @@ immed_double_const (HOST_WIDE_INT i0, HOST_WIDE_INT i1, enum machine_mode mode)
 
      1) If GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT, then we use
 	gen_int_mode.
-     2) GET_MODE_BITSIZE (mode) == 2 * HOST_BITS_PER_WIDE_INT, but the value of
-	the integer fits into HOST_WIDE_INT anyway (i.e., i1 consists only
-	from copies of the sign bit, and sign of i0 and i1 are the same),  then
-	we return a CONST_INT for i0.
+     2) If the value of the integer fits into HOST_WIDE_INT anyway
+        (i.e., i1 consists only from copies of the sign bit, and sign
+	of i0 and i1 are the same), then we return a CONST_INT for i0.
      3) Otherwise, we create a CONST_DOUBLE for i0 and i1.  */
   if (mode != VOIDmode)
     {
diff --git a/gcc/explow.c b/gcc/explow.c
index 2fae1a1..6284d61 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -96,6 +96,9 @@ plus_constant (rtx x, HOST_WIDE_INT c)
   switch (code)
     {
     case CONST_INT:
+      if (GET_MODE_BITSIZE (mode) > HOST_WIDE_INT)
+	/* Punt for now.  */
+	goto overflow;
       return GEN_INT (INTVAL (x) + c);
 
     case CONST_DOUBLE:
@@ -103,10 +106,14 @@ plus_constant (rtx x, HOST_WIDE_INT c)
 	unsigned HOST_WIDE_INT l1 = CONST_DOUBLE_LOW (x);
 	HOST_WIDE_INT h1 = CONST_DOUBLE_HIGH (x);
 	unsigned HOST_WIDE_INT l2 = c;
-	HOST_WIDE_INT h2 = c < 0 ? ~0 : 0;
+	HOST_WIDE_INT h2 = c < 0 ? ~(HOST_WIDE_INT)0 : 0;
 	unsigned HOST_WIDE_INT lv;
 	HOST_WIDE_INT hv;
 
+	if (GET_MODE_BITSIZE (mode) > 2*HOST_WIDE_INT)
+	  /* Punt for now.  */
+	  goto overflow;
+
 	add_double (l1, h1, l2, h2, &lv, &hv);
 
 	return immed_double_const (lv, hv, VOIDmode);
@@ -141,6 +148,9 @@ plus_constant (rtx x, HOST_WIDE_INT c)
       break;
 
     case PLUS:
+      if (GET_MODE_BITSIZE (mode) > HOST_WIDE_INT)
+	/* Punt for now.  */
+	goto overflow;
       /* The interesting case is adding the integer to a sum.
 	 Look for constant term in the sum and combine
 	 with C.  For an integer constant term, we make a combined
@@ -185,6 +195,7 @@ plus_constant (rtx x, HOST_WIDE_INT c)
       break;
     }
 
+ overflow:
   if (c != 0)
     x = gen_rtx_PLUS (mode, x, GEN_INT (c));
 
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index ce4eab4..37e46b1 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -101,6 +101,7 @@ mode_signbit_p (enum machine_mode mode, const_rtx x)
 
   if (width < HOST_BITS_PER_WIDE_INT)
     val &= ((unsigned HOST_WIDE_INT) 1 << width) - 1;
+  /* FIXME: audit for TImode and wider correctness.  */
   return val == ((unsigned HOST_WIDE_INT) 1 << (width - 1));
 }
 

@@ -1227,7 +1228,7 @@ simplify_const_unary_operation (enum rtx_code code, enum machine_mode mode,
 	    return 0;
 	}
       else if (GET_MODE_BITSIZE (op_mode) >= HOST_BITS_PER_WIDE_INT * 2)
-	;
+	return 0;
       else
 	hv = 0, lv &= GET_MODE_MASK (op_mode);
 
@@ -4987,6 +4988,7 @@ simplify_immed_subreg (enum machine_mode outermode, rtx op,
 	case CONST_DOUBLE:
 	  if (GET_MODE (el) == VOIDmode)
 	    {
+	      unsigned char extend = 0;
 	      /* If this triggers, someone should have generated a
 		 CONST_INT instead.  */
 	      gcc_assert (elem_bitsize > HOST_BITS_PER_WIDE_INT);
@@ -4999,13 +5001,15 @@ simplify_immed_subreg (enum machine_mode outermode, rtx op,
 		    = CONST_DOUBLE_HIGH (el) >> (i - HOST_BITS_PER_WIDE_INT);
 		  i += value_bit;
 		}
-	      /* It shouldn't matter what's done here, so fill it with
-		 zero.  */
+
+	      if (CONST_DOUBLE_HIGH (el) >> (HOST_BITS_PER_WIDE_INT-1))
+		extend = -1;
 	      for (; i < elem_bitsize; i += value_bit)
-		*vp++ = 0;
+		*vp++ = extend;
 	    }
 	  else
 	    {
+	      unsigned char extend = 0;
 	      long tmp[max_bitsize / 32];
 	      int bitsize = GET_MODE_BITSIZE (GET_MODE (el));
 
@@ -5030,10 +5034,10 @@ simplify_immed_subreg (enum machine_mode outermode, rtx op,
 		  *vp++ = tmp[ibase / 32] >> i % 32;
 		}
 
-	      /* It shouldn't matter what's done here, so fill it with
-		 zero.  */
+	      if (CONST_DOUBLE_HIGH (el) >> (HOST_BITS_PER_WIDE_INT-1))
+		extend = -1;
 	      for (; i < elem_bitsize; i += value_bit)
-		*vp++ = 0;
+		*vp++ = extend;
 	    }
 	  break;