Patchwork remove wrong code in immed_double_const

login
register
mail settings
Submitter Mike Stump
Date March 21, 2012, 9:35 p.m.
Message ID <DF6339D4-BFF6-45FA-A5E0-8141F0BD4199@comcast.net>
Download mbox | patch
Permalink /patch/148068/
State New
Headers show

Comments

Mike Stump - March 21, 2012, 9:35 p.m.
On Mar 21, 2012, at 6:17 AM, Richard Sandiford wrote:
> Sounds good.

Cool, a path forward.

> For this I think we should make plus_constant a wrapper:

Ah, yes, much better, thanks.  I'd expanded the comments on plus_constant_mode so that one might have a better idea why the code is that way and put in a TODO, so that people have an idea of what direction the code wants to move.

> I don't think it's a good idea to punt to a PLUS though.

Fixed, thanks for the code.

> Nicely, add_double returns true on overflow, so I think
> we should replace the punt with:

Ah, yes, nice, fixed.

> For this I think we should change the recursive CONSTANT_P call
> to use plus_constant_mode

Done.

> and we can remove the special CONST_INT case.

Ok, ah, yes, because the recursive call will already combine them, done.

>>   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));
> 
> We've already returned false in that case though.

Ah, I saw it, but missed it anyway, thanks for the pointer, fixed.

> I'm happy for this function to be left as-is, but we could instead add a comment like:
> 
>    /* FIXME: We don't yet have a representation for wider modes.  */

Done.

>> @@ -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);
> 
> Should keep the current behaviour for == HOST_BITS_PER_WIDE_INT * 2.


> I'd slightly prefer an assert rather than a "return false", but I won't
> argue if another maintainer agrees with the return.

Ah, yes, I agree an assert would be better, as really, no one should ask for an unsigned conversion to floating point on a value that is negative, more likely it is just a spot we missed adding an assert to earlier and probably wants a larger constant that can represent a large unsigned number.  Fixed.

> This is changing the real case, where sign extension doesn't make
> much sense.

I'm not sure I followed.  Do you want me to remove the change for the second case, leave it as it, or something else?  I've left it as I had modified it.

> I think the expand_mult CONST_DOUBLE code needs changing

Agreed.  Fixed.  I preserve the code for smaller modes, and for small enough shift amounts. 1<<67 for example, or any mode, is still handled.  For large enough things, we just don't return the shift.

> Same for:

Fixed, in same way as previous case.

> simplify_const_unary_operation needs to check for overflow
> when handling 2-HWI arithmetic, since we can no longer assume
> that the result is <= 2 HWIs in size.  E.g.:
> 
> 	case NEG:
> 	  neg_double (l1, h1, &lv, &hv);
> 	  break;
> 
> should probably be:
> 
> 	case NEG:
> 	  if (neg_double (l1, h1, &lv, &hv))
>            gcc_assert (GET_MODE_BITSIZE (mode) <= 2 * HOST_WIDE_INT);
> 	  break;

Are you talking about the block of code inside:

  /* We can do some operations on integer CONST_DOUBLEs.  Also allow                                                                              
     for a DImode operation on a CONST_INT.  */
  else if (GET_MODE (op) == VOIDmode
           && width <= HOST_BITS_PER_WIDE_INT * 2

?  If so, we already know that the width is <= HOST_BITS_PER_WIDE_INT * 2.  :-)

Thanks for spotting the bits I missed.


Current patch:
Richard Sandiford - March 22, 2012, 10:16 a.m.
Mike Stump <mikestump@comcast.net> writes:
>> This is changing the real case, where sign extension doesn't make
>> much sense.
>
> I'm not sure I followed.  Do you want me to remove the change for the
> second case, leave it as it, or something else?  I've left it as I had
> modified it.

Sorry, meant we should leave the svn version as it is.  The new docs
(rightly) only mention sign-extension for integer modes, so the comment
about it not mattering for FP modes is still correct.  (In principle
I'd prefer to replace it with an assert, but that's a separate change.
Nothing we're doing here should change whether the FP path gets executed.)

>> simplify_const_unary_operation needs to check for overflow
>> when handling 2-HWI arithmetic, since we can no longer assume
>> that the result is <= 2 HWIs in size.  E.g.:
>> 
>> 	case NEG:
>> 	  neg_double (l1, h1, &lv, &hv);
>> 	  break;
>> 
>> should probably be:
>> 
>> 	case NEG:
>> 	  if (neg_double (l1, h1, &lv, &hv))
>>            gcc_assert (GET_MODE_BITSIZE (mode) <= 2 * HOST_WIDE_INT);
>> 	  break;
>
> Are you talking about the block of code inside:
>
>   /* We can do some operations on integer CONST_DOUBLEs.  Also allow                                                                              
>      for a DImode operation on a CONST_INT.  */
>   else if (GET_MODE (op) == VOIDmode
>            && width <= HOST_BITS_PER_WIDE_INT * 2
>
> ?

Heh. it seems so :-)  Never mind that bit then.

> diff --git a/gcc/explow.c b/gcc/explow.c
> index 2fae1a1..c94ad25 100644
> --- a/gcc/explow.c
> +++ b/gcc/explow.c
> @@ -73,14 +73,20 @@ trunc_int_for_mode (HOST_WIDE_INT c, enum machine_mode mode)
>    return c;
>  }
>  
> -/* Return an rtx for the sum of X and the integer C.  */
> +/* Return an rtx for the sum of X and the integer C, given that X has
> +   mode MODE.  This routine should be used instead of plus_constant
> +   when they want to ensure that addition happens in a particular
> +   mode, which is necessary when x can be a VOIDmode CONST_INT or
> +   CONST_DOUBLE and the width of the constant is smaller than the
> +   width of the expression.  */

I think this should be s/is smaller than/is different from/.
We're in trouble whenever the width of the HWIs is different
from the width of the constant they represent.

> @@ -103,11 +123,16 @@ 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 ? -1 : 0;
>  	unsigned HOST_WIDE_INT lv;
>  	HOST_WIDE_INT hv;
>  
> -	add_double (l1, h1, l2, h2, &lv, &hv);
> +	if (add_double_with_sign (l1, h1, l2, h2, &lv, &hv, false))
> +	  if (GET_MODE_BITSIZE (mode) > 2*HOST_BITS_PER_WIDE_INT)
> +	    /* Sorry, we have no way to represent overflows this
> +	       wide.  To fix, add constant support wider than
> +	       CONST_DOUBLE.  */
> +	    gcc_assert (0);

Should be:

	if (add_double_with_sign (l1, h1, l2, h2, &lv, &hv, false))
	  /* Sorry, we have no way to represent overflows this
	     wide.  To fix, add constant support wider than
	     CONST_DOUBLE.  */
	  gcc_assert (GET_MODE_BITSIZE (mode) > 2 * HOST_BITS_PER_WIDE_INT)

(note spacing).

> @@ -121,8 +146,8 @@ plus_constant (rtx x, HOST_WIDE_INT c)
>  	{
>  	  tem
>  	    = force_const_mem (GET_MODE (x),
> -			       plus_constant (get_pool_constant (XEXP (x, 0)),
> -					      c));
> +			       plus_constant_mode (mode, get_pool_constant (XEXP (x, 0)),
> +						   c));
>  	  if (memory_address_p (GET_MODE (tem), XEXP (tem, 0)))
>  	    return tem;
>  	}

Nitlet, but line is wider than 80 chars.  Probably easiest fix is:

	  tem = plus_constant_mode (mode, get_pool_constant (XEXP (x, 0)), c);
	  tem = force_const_mem (GET_MODE (x), tem);

> diff --git a/gcc/expmed.c b/gcc/expmed.c
> index 3507dad..2361b7e 100644
> --- a/gcc/expmed.c
> +++ b/gcc/expmed.c
> @@ -3122,9 +3122,11 @@ expand_mult (enum machine_mode mode, rtx op0, rtx op1, rtx target,
>  	    {
>  	      int shift = floor_log2 (CONST_DOUBLE_HIGH (op1))
>  			  + HOST_BITS_PER_WIDE_INT;
> -	      return expand_shift (LSHIFT_EXPR, mode, op0,
> -				   build_int_cst (NULL_TREE, shift),
> -				   target, unsignedp);
> +	      if (shift < 2*HOST_BITS_PER_WIDE_INT-1
> +		  || GET_MODE_BITSIZE (mode) <= 2*HOST_BITS_PER_WIDE_INT)

Missing spaces around binary operators.

> @@ -1219,15 +1220,10 @@ simplify_const_unary_operation (enum rtx_code code, enum machine_mode mode,
>        else
>  	lv = CONST_DOUBLE_LOW (op),  hv = CONST_DOUBLE_HIGH (op);
>  
> -      if (op_mode == VOIDmode)
> -	{
> -	  /* We don't know how to interpret negative-looking numbers in
> -	     this case, so don't try to fold those.  */
> -	  if (hv < 0)
> -	    return 0;
> -	}
> -      else if (GET_MODE_BITSIZE (op_mode) >= HOST_BITS_PER_WIDE_INT * 2)
> -	;
> +      if (op_mode == VOIDmode
> +	  || GET_MODE_BITSIZE (op_mode) >= HOST_BITS_PER_WIDE_INT * 2)
> +	/* We should never get a negative number.  */
> +	gcc_assert (hv >= 0);
>        else
>  	hv = 0, lv &= GET_MODE_MASK (op_mode);
>  

Sorry, with this bit, I meant that the current svn code is correct
for GET_MODE_BITSIZE (op_mode) == HOST_BITS_PER_WIDE_INT * 2.
In that case, hv < 0 can just mean that we have a uint128_t
(or whatever) whose high bit happens to be set.  I think it
should be something like:

      if (op_mode == VOIDmode
	  || GET_MODE_BITSIZE (op_mode) > HOST_BITS_PER_WIDE_INT * 2)
	/* We should never get a negative number.  */
	gcc_assert (hv >= 0);
      else if (GET_MODE_BITSIZE (op_mode) <= HOST_BITS_PER_WIDE_INT)
	hv = 0, lv &= GET_MODE_MASK (op_mode);

> @@ -2214,7 +2210,9 @@ simplify_binary_operation_1 (enum rtx_code code, enum machine_mode mode,
>  	      || GET_MODE_CLASS (GET_MODE (trueop1)) == MODE_INT)
>  	  && GET_MODE (op0) == mode
>  	  && CONST_DOUBLE_LOW (trueop1) == 0
> -	  && (val = exact_log2 (CONST_DOUBLE_HIGH (trueop1))) >= 0)
> +	  && (val = exact_log2 (CONST_DOUBLE_HIGH (trueop1))) >= 0
> +	  && (val < 2*HOST_BITS_PER_WIDE_INT-1
> +	      || GET_MODE_BITSIZE (mode) <= 2*HOST_BITS_PER_WIDE_INT))
>  	return simplify_gen_binary (ASHIFT, mode, op0,
>  				    GEN_INT (val + HOST_BITS_PER_WIDE_INT));
>  

Missing spaces around binary operators.

OK with those changes as far as the RTL optimisations go.  I'm happy
with the rest too, but despite all this fuss, I can't approve the
immed_double_const change itself.  Sounds like Richard G would be
willing though.

Richard
Richard Sandiford - March 22, 2012, 10:24 a.m.
Richard Sandiford <rdsandiford@googlemail.com> writes:
> Should be:
>
> 	if (add_double_with_sign (l1, h1, l2, h2, &lv, &hv, false))
> 	  /* Sorry, we have no way to represent overflows this
> 	     wide.  To fix, add constant support wider than
> 	     CONST_DOUBLE.  */
> 	  gcc_assert (GET_MODE_BITSIZE (mode) > 2 * HOST_BITS_PER_WIDE_INT)

Er, I did of course mean:

	  gcc_assert (GET_MODE_BITSIZE (mode) <= 2 * HOST_BITS_PER_WIDE_INT)

:-)

Richard
Michael Matz - March 22, 2012, 2:12 p.m.
Hi,

On Wed, 21 Mar 2012, Mike Stump wrote:

> --- 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.

I see that you didn't remove the assert as part of this patch.  I'd like 
to see what you like to do to this routine once the rest goes in.  In 
particular I don't think just removing the assert will be enough, at the 
very least the block comment should be saying something about what the 
routine exactly does (or doesn't do) for modes where the two HWI arguments 
can't specify all bits.

My point is, for large modes i0 and i1 will only specify the low 2*HWIbits 
bits.  Something needs to document what the upper bits will be (be they 
implicit or explicit), otherwise people reading the comment will always 
wonder what exactly is supposed to happen.  I'm not 100% sure what it 
should say, though.  Probably the interpretation of the upper bits depends 
on the users of the so generated CONST_DOUBLEs.


Ciao,
Michael.
Mike Stump - March 22, 2012, 6:55 p.m.
On Mar 22, 2012, at 7:12 AM, Michael Matz wrote:
> I see that you didn't remove the assert as part of this patch.

I'll include that in my next patch.

> I'd like  to see what you like to do to this routine once the rest goes in.  In 
> particular I don't think just removing the assert will be enough, at the 
> very least the block comment should be saying something about what the 
> routine exactly does (or doesn't do) for modes where the two HWI arguments 
> can't specify all bits.

I think the best approach is to refine the spec:

/* Return a CONST_DOUBLE or CONST_INT for a value specified as a pair                                                                                             
   of ints: I0 is the low-order word and I1 is the high-order word.                                                                                               
   The value is a signed value, with the high bit of i1 being the sign                                                                                            
   bit.  Do not use this routine for non-integer modes; convert to                                                                                                
   REAL_VALUE_TYPE and use CONST_DOUBLE_FROM_REAL_VALUE.  */

I think this then exactly matches CONST_DOUBLE semantics.

Patch

diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texiindex 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..c94ad25 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -73,14 +73,20 @@  trunc_int_for_mode (HOST_WIDE_INT c, enum machine_mode mode)
   return c;
 }
 
-/* Return an rtx for the sum of X and the integer C.  */
+/* Return an rtx for the sum of X and the integer C, given that X has
+   mode MODE.  This routine should be used instead of plus_constant
+   when they want to ensure that addition happens in a particular
+   mode, which is necessary when x can be a VOIDmode CONST_INT or
+   CONST_DOUBLE and the width of the constant is smaller than the
+   width of the expression.  */
+/* TODO: All callers of plus_constant should migrate to this routine,
+   and once they do, we can assert that mode is not VOIDmode.  */
 
 rtx
-plus_constant (rtx x, HOST_WIDE_INT c)
+plus_constant_mode (enum machine_mode mode, rtx x, HOST_WIDE_INT c)
 {
   RTX_CODE code;
   rtx y;
-  enum machine_mode mode;
   rtx tem;
   int all_constant = 0;
 
@@ -90,12 +96,26 @@  plus_constant (rtx x, HOST_WIDE_INT c)
  restart:
 
   code = GET_CODE (x);
-  mode = GET_MODE (x);
   y = x;
 
   switch (code)
     {
     case CONST_INT:
+      if (GET_MODE_BITSIZE (mode) > HOST_BITS_PER_WIDE_INT)
+	{
+	  unsigned HOST_WIDE_INT l1 = INTVAL (x);
+	  HOST_WIDE_INT h1 = (l1 >> (HOST_BITS_PER_WIDE_INT-1)) ? -1 : 0;
+	  unsigned HOST_WIDE_INT l2 = c;
+	  HOST_WIDE_INT h2 = c < 0 ? -1 : 0;
+	  unsigned HOST_WIDE_INT lv;
+	  HOST_WIDE_INT hv;
+
+	  if (add_double_with_sign (l1, h1, l2, h2, &lv, &hv, false))
+	    gcc_unreachable ();
+
+	  return immed_double_const (lv, hv, VOIDmode);
+	}
+
       return GEN_INT (INTVAL (x) + c);
 
     case CONST_DOUBLE:
@@ -103,11 +123,16 @@  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 ? -1 : 0;
 	unsigned HOST_WIDE_INT lv;
 	HOST_WIDE_INT hv;
 
-	add_double (l1, h1, l2, h2, &lv, &hv);
+	if (add_double_with_sign (l1, h1, l2, h2, &lv, &hv, false))
+	  if (GET_MODE_BITSIZE (mode) > 2*HOST_BITS_PER_WIDE_INT)
+	    /* Sorry, we have no way to represent overflows this
+	       wide.  To fix, add constant support wider than
+	       CONST_DOUBLE.  */
+	    gcc_assert (0);
 
 	return immed_double_const (lv, hv, VOIDmode);
       }
@@ -121,8 +146,8 @@  plus_constant (rtx x, HOST_WIDE_INT c)
 	{
 	  tem
 	    = force_const_mem (GET_MODE (x),
-			       plus_constant (get_pool_constant (XEXP (x, 0)),
-					      c));
+			       plus_constant_mode (mode, get_pool_constant (XEXP (x, 0)),
+						   c));
 	  if (memory_address_p (GET_MODE (tem), XEXP (tem, 0)))
 	    return tem;
 	}
@@ -141,31 +166,17 @@  plus_constant (rtx x, HOST_WIDE_INT c)
       break;
 
     case PLUS:
-      /* 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
-	 integer.  For a constant term that is not an explicit integer,
-	 we cannot really combine, but group them together anyway.
-
-	 Restart or use a recursive call in case the remaining operand is
-	 something that we handle specially, such as a SYMBOL_REF.
+      /* 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 or a constant term that is not an
+	 explicit integer, we combine or group them together anyway.
 
 	 We may not immediately return from the recursive call here, lest
 	 all_constant gets lost.  */
 
-      if (CONST_INT_P (XEXP (x, 1)))
+      if (CONSTANT_P (XEXP (x, 1)))
 	{
-	  c += INTVAL (XEXP (x, 1));
-
-	  if (GET_MODE (x) != VOIDmode)
-	    c = trunc_int_for_mode (c, GET_MODE (x));
-
-	  x = XEXP (x, 0);
-	  goto restart;
-	}
-      else if (CONSTANT_P (XEXP (x, 1)))
-	{
-	  x = gen_rtx_PLUS (mode, XEXP (x, 0), plus_constant (XEXP (x, 1), c));
+	  x = gen_rtx_PLUS (mode, XEXP (x, 0), plus_constant_mode (mode, XEXP (x, 1), c));
 	  c = 0;
 	}
       else if (find_constant_term_loc (&y))
@@ -175,7 +186,7 @@  plus_constant (rtx x, HOST_WIDE_INT c)
 	  rtx copy = copy_rtx (x);
 	  rtx *const_loc = find_constant_term_loc (&copy);
 
-	  *const_loc = plus_constant (*const_loc, c);
+	  *const_loc = plus_constant_mode (mode, *const_loc, c);
 	  x = copy;
 	  c = 0;
 	}
@@ -195,6 +206,14 @@  plus_constant (rtx x, HOST_WIDE_INT c)
   else
     return x;
 }
+
+/* Return an rtx for the sum of X and the integer C.  */
+
+rtx
+plus_constant (rtx x, HOST_WIDE_INT c)
+{
+  return plus_constant_mode (GET_MODE (x), x, c);
+}
 

 /* If X is a sum, return a new sum like X but lacking any constant terms.
    Add all the removed constant terms into *CONSTPTR.
diff --git a/gcc/expmed.c b/gcc/expmed.c
index 3507dad..2361b7e 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -3122,9 +3122,11 @@  expand_mult (enum machine_mode mode, rtx op0, rtx op1, rtx target,
 	    {
 	      int shift = floor_log2 (CONST_DOUBLE_HIGH (op1))
 			  + HOST_BITS_PER_WIDE_INT;
-	      return expand_shift (LSHIFT_EXPR, mode, op0,
-				   build_int_cst (NULL_TREE, shift),
-				   target, unsignedp);
+	      if (shift < 2*HOST_BITS_PER_WIDE_INT-1
+		  || GET_MODE_BITSIZE (mode) <= 2*HOST_BITS_PER_WIDE_INT)
+		return expand_shift (LSHIFT_EXPR, mode, op0,
+				     build_int_cst (NULL_TREE, shift),
+				     target, unsignedp);
 	    }
 	}
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 66f2755..9d1a28e 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1586,6 +1586,7 @@  extern int ceil_log2 (unsigned HOST_WIDE_INT);
 /* In explow.c */
 extern HOST_WIDE_INT trunc_int_for_mode	(HOST_WIDE_INT, enum machine_mode);
 extern rtx plus_constant (rtx, HOST_WIDE_INT);
+extern rtx plus_constant_mode (enum machine_mode, rtx, HOST_WIDE_INT);
 
 /* In rtl.c */
 extern rtx rtx_alloc_stat (RTX_CODE MEM_STAT_DECL);
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index ce4eab4..ff838a8 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -97,6 +97,7 @@  mode_signbit_p (enum machine_mode mode, const_rtx x)
       width -= HOST_BITS_PER_WIDE_INT;
     }
   else
+    /* FIXME: We don't yet have a representation for wider modes.  */
     return false;
 
   if (width < HOST_BITS_PER_WIDE_INT)
@@ -1219,15 +1220,10 @@  simplify_const_unary_operation (enum rtx_code code, enum machine_mode mode,
       else
 	lv = CONST_DOUBLE_LOW (op),  hv = CONST_DOUBLE_HIGH (op);
 
-      if (op_mode == VOIDmode)
-	{
-	  /* We don't know how to interpret negative-looking numbers in
-	     this case, so don't try to fold those.  */
-	  if (hv < 0)
-	    return 0;
-	}
-      else if (GET_MODE_BITSIZE (op_mode) >= HOST_BITS_PER_WIDE_INT * 2)
-	;
+      if (op_mode == VOIDmode
+	  || GET_MODE_BITSIZE (op_mode) >= HOST_BITS_PER_WIDE_INT * 2)
+	/* We should never get a negative number.  */
+	gcc_assert (hv >= 0);
       else
 	hv = 0, lv &= GET_MODE_MASK (op_mode);
 
@@ -2214,7 +2210,9 @@  simplify_binary_operation_1 (enum rtx_code code, enum machine_mode mode,
 	      || GET_MODE_CLASS (GET_MODE (trueop1)) == MODE_INT)
 	  && GET_MODE (op0) == mode
 	  && CONST_DOUBLE_LOW (trueop1) == 0
-	  && (val = exact_log2 (CONST_DOUBLE_HIGH (trueop1))) >= 0)
+	  && (val = exact_log2 (CONST_DOUBLE_HIGH (trueop1))) >= 0
+	  && (val < 2*HOST_BITS_PER_WIDE_INT-1
+	      || GET_MODE_BITSIZE (mode) <= 2*HOST_BITS_PER_WIDE_INT))
 	return simplify_gen_binary (ASHIFT, mode, op0,
 				    GEN_INT (val + HOST_BITS_PER_WIDE_INT));
 
@@ -4987,6 +4985,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 +4998,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 +5031,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;