diff mbox

RFA: Use gen_int_mode in plus_constant

Message ID 87mwrosmye.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com
State New
Headers show

Commit Message

Richard Sandiford May 21, 2013, 9:26 a.m. UTC 
Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
> On 21/05/13 10:39, Richard Sandiford wrote:
>> Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
>>> On 30/04/13 16:56, Richard Sandiford wrote:
>>>> This patch fixes out the GEN_INT/gen_int_mode that Richard pointed out
>>>> in the wide-int review.  It also passes "mode" rather than "VOIDmode"
>>>> to immed_double_int_const.  (As discussed in that thread, the latter
>>>> change shouldn't make any difference in practice, but is still more
>>>> correct in principle.)
>>>>
>>>> Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?
>>>>
>>>> Richard
>>>>
>>>> gcc/
>>>> 	* explow.c (plus_constant): Pass "mode" to immed_double_int_const.
>>>> 	Use gen_int_mode rather than GEN_INT.
>>>>
>>>> Index: gcc/explow.c
>>>> ===================================================================
>>>> --- gcc/explow.c	2013-02-25 09:41:58.000000000 +0000
>>>> +++ gcc/explow.c	2013-04-30 15:52:57.270362112 +0100
>>>> @@ -106,10 +106,10 @@ plus_constant (enum machine_mode mode, r
>>>>  	  if (overflow)
>>>>  	    gcc_unreachable ();
>>>>
>>>> -	  return immed_double_int_const (v, VOIDmode);
>>>> +	  return immed_double_int_const (v, mode);
>>>>  	}
>>>>
>>>> -      return GEN_INT (INTVAL (x) + c);
>>>> +      return gen_int_mode (INTVAL (x) + c, mode);
>>>
>>> This calls trunc_int_for_mode which fails for mode == VOIDmode.
>>> On s390x gcc.c-torture/compile/20021008-1.c fails due to this.
>> 
>> But it's invalid to pass mode == VOIDmode to plus_constant too.
>> Which caller is causing trouble?
>> 
>> Thanks,
>> Richard
>> 
>
> Hi Richard,
>
> the call comes from reload when checking a const_int memory address.

Ah, thanks.  Could you give the patch below a go?

Richard


gcc/
	* recog.c (offsettable_address_addr_space_p): Fix calculation of
	address mode.  Move pointer mode initialization to the same place.

Comments

Andreas Krebbel May 21, 2013, 1:27 p.m. UTC | #1 
On 21/05/13 11:26, Richard Sandiford wrote:
> Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
>> On 21/05/13 10:39, Richard Sandiford wrote:
>>> Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
>>>> On 30/04/13 16:56, Richard Sandiford wrote:
>>>>> This patch fixes out the GEN_INT/gen_int_mode that Richard pointed out
>>>>> in the wide-int review.  It also passes "mode" rather than "VOIDmode"
>>>>> to immed_double_int_const.  (As discussed in that thread, the latter
>>>>> change shouldn't make any difference in practice, but is still more
>>>>> correct in principle.)
>>>>>
>>>>> Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?
>>>>>
>>>>> Richard
>>>>>
>>>>> gcc/
>>>>> 	* explow.c (plus_constant): Pass "mode" to immed_double_int_const.
>>>>> 	Use gen_int_mode rather than GEN_INT.
>>>>>
>>>>> Index: gcc/explow.c
>>>>> ===================================================================
>>>>> --- gcc/explow.c	2013-02-25 09:41:58.000000000 +0000
>>>>> +++ gcc/explow.c	2013-04-30 15:52:57.270362112 +0100
>>>>> @@ -106,10 +106,10 @@ plus_constant (enum machine_mode mode, r
>>>>>  	  if (overflow)
>>>>>  	    gcc_unreachable ();
>>>>>
>>>>> -	  return immed_double_int_const (v, VOIDmode);
>>>>> +	  return immed_double_int_const (v, mode);
>>>>>  	}
>>>>>
>>>>> -      return GEN_INT (INTVAL (x) + c);
>>>>> +      return gen_int_mode (INTVAL (x) + c, mode);
>>>>
>>>> This calls trunc_int_for_mode which fails for mode == VOIDmode.
>>>> On s390x gcc.c-torture/compile/20021008-1.c fails due to this.
>>>
>>> But it's invalid to pass mode == VOIDmode to plus_constant too.
>>> Which caller is causing trouble?
>>>
>>> Thanks,
>>> Richard
>>>
>>
>> Hi Richard,
>>
>> the call comes from reload when checking a const_int memory address.
> 
> Ah, thanks.  Could you give the patch below a go?
> 
> Richard
> 
> 
> gcc/
> 	* recog.c (offsettable_address_addr_space_p): Fix calculation of
> 	address mode.  Move pointer mode initialization to the same place.

Thanks! This fixed the failure (and others). Bootstrapped on s390x - no regressions:

< FAIL: gcc.c-torture/compile/20021008-1.c  -O1  (internal compiler error)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O1  (test for excess errors)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O2  (internal compiler error)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O2  (test for excess errors)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O3 -fomit-frame-pointer  (internal compiler error)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O3 -fomit-frame-pointer  (test for excess errors)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O3 -g  (internal compiler error)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O3 -g  (test for excess errors)
< FAIL: gcc.c-torture/compile/20021008-1.c  -Os  (internal compiler error)
< FAIL: gcc.c-torture/compile/20021008-1.c  -Os  (test for excess errors)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O2 -flto -fno-use-linker-plugin -flto-partition=none
(internal compiler error)
< FAIL: gcc.c-torture/compile/20021008-1.c  -O2 -flto -fno-use-linker-plugin -flto-partition=none
(test for excess errors)
< FAIL: gcc.c-torture/execute/pr31448-2.c compilation,  -O1  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448-2.c execution,  -O1
< FAIL: gcc.c-torture/execute/pr31448-2.c compilation,  -O2  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448-2.c execution,  -O2
< FAIL: gcc.c-torture/execute/pr31448-2.c compilation,  -O3 -fomit-frame-pointer  (internal compiler
error)
< UNRESOLVED: gcc.c-torture/execute/pr31448-2.c execution,  -O3 -fomit-frame-pointer
< FAIL: gcc.c-torture/execute/pr31448-2.c compilation,  -O3 -g  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448-2.c execution,  -O3 -g
< FAIL: gcc.c-torture/execute/pr31448-2.c compilation,  -Os  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448-2.c execution,  -Os
< FAIL: gcc.c-torture/execute/pr31448-2.c compilation,  -Og -g  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448-2.c execution,  -Og -g
< FAIL: gcc.c-torture/execute/pr31448-2.c compilation,  -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448-2.c execution,  -O2 -flto -fno-use-linker-plugin
-flto-partition=none
< FAIL: gcc.c-torture/execute/pr31448.c compilation,  -O1  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448.c execution,  -O1
< FAIL: gcc.c-torture/execute/pr31448.c compilation,  -O2  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448.c execution,  -O2
< FAIL: gcc.c-torture/execute/pr31448.c compilation,  -O3 -fomit-frame-pointer  (internal compiler
error)
< UNRESOLVED: gcc.c-torture/execute/pr31448.c execution,  -O3 -fomit-frame-pointer
< FAIL: gcc.c-torture/execute/pr31448.c compilation,  -O3 -g  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448.c execution,  -O3 -g
< FAIL: gcc.c-torture/execute/pr31448.c compilation,  -Os  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448.c execution,  -Os
< FAIL: gcc.c-torture/execute/pr31448.c compilation,  -Og -g  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448.c execution,  -Og -g
< FAIL: gcc.c-torture/execute/pr31448.c compilation,  -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (internal compiler error)
< UNRESOLVED: gcc.c-torture/execute/pr31448.c execution,  -O2 -flto -fno-use-linker-plugin
-flto-partition=none
532,533c492,493
< # of expected passes          83431
< # of unexpected failures      342
---
> # of expected passes          83465
> # of unexpected failures      316
536c496
< # of unresolved testcases     24
---
> # of unresolved testcases     10
578,579d537
< FAIL: g++.dg/tree-ssa/copyprop.C (internal compiler error)
< FAIL: g++.dg/tree-ssa/copyprop.C (test for excess errors)
583,584c541
< # of expected passes          52505
< # of unexpected failures      2
---
> # of expected passes          52506



> 
> Index: gcc/recog.c
> ===================================================================
> --- gcc/recog.c	2013-04-29 12:38:00.000000000 +0100
> +++ gcc/recog.c	2013-05-21 10:21:24.553671961 +0100
> @@ -1953,9 +1953,6 @@ offsettable_address_addr_space_p (int st
>      (strictp ? strict_memory_address_addr_space_p
>  	     : memory_address_addr_space_p);
>    unsigned int mode_sz = GET_MODE_SIZE (mode);
> -#ifdef POINTERS_EXTEND_UNSIGNED
> -  enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as);
> -#endif
> 
>    if (CONSTANT_ADDRESS_P (y))
>      return 1;
> @@ -1966,6 +1963,13 @@ offsettable_address_addr_space_p (int st
>    if (mode_dependent_address_p (y, as))
>      return 0;
> 
> +  enum machine_mode address_mode = GET_MODE (y);
> +  if (address_mode == VOIDmode)
> +    address_mode = targetm.addr_space.address_mode (as);
> +#ifdef POINTERS_EXTEND_UNSIGNED
> +  enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as);
> +#endif
> +
>    /* ??? How much offset does an offsettable BLKmode reference need?
>       Clearly that depends on the situation in which it's being used.
>       However, the current situation in which we test 0xffffffff is
> @@ -1981,7 +1985,7 @@ offsettable_address_addr_space_p (int st
>        int good;
> 
>        y1 = *y2;
> -      *y2 = plus_constant (GET_MODE (y), *y2, mode_sz - 1);
> +      *y2 = plus_constant (address_mode, *y2, mode_sz - 1);
>        /* Use QImode because an odd displacement may be automatically invalid
>  	 for any wider mode.  But it should be valid for a single byte.  */
>        good = (*addressp) (QImode, y, as);
> @@ -2002,20 +2006,20 @@ offsettable_address_addr_space_p (int st
>    if (GET_CODE (y) == LO_SUM
>        && mode != BLKmode
>        && mode_sz <= GET_MODE_ALIGNMENT (mode) / BITS_PER_UNIT)
> -    z = gen_rtx_LO_SUM (GET_MODE (y), XEXP (y, 0),
> -			plus_constant (GET_MODE (y), XEXP (y, 1),
> +    z = gen_rtx_LO_SUM (address_mode, XEXP (y, 0),
> +			plus_constant (address_mode, XEXP (y, 1),
>  				       mode_sz - 1));
>  #ifdef POINTERS_EXTEND_UNSIGNED
>    /* Likewise for a ZERO_EXTEND from pointer_mode.  */
>    else if (POINTERS_EXTEND_UNSIGNED > 0
>  	   && GET_CODE (y) == ZERO_EXTEND
>  	   && GET_MODE (XEXP (y, 0)) == pointer_mode)
> -    z = gen_rtx_ZERO_EXTEND (GET_MODE (y),
> +    z = gen_rtx_ZERO_EXTEND (address_mode,
>  			     plus_constant (pointer_mode, XEXP (y, 0),
>  					    mode_sz - 1));
>  #endif
>    else
> -    z = plus_constant (GET_MODE (y), y, mode_sz - 1);
> +    z = plus_constant (address_mode, y, mode_sz - 1);
> 
>    /* Use QImode because an odd displacement may be automatically invalid
>       for any wider mode.  But it should be valid for a single byte.  */
>
Richard Sandiford May 22, 2013, 9:02 a.m. UTC | #2 
Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
> On 21/05/13 11:26, Richard Sandiford wrote:
>> gcc/
>> 	* recog.c (offsettable_address_addr_space_p): Fix calculation of
>> 	address mode.  Move pointer mode initialization to the same place.
>
> Thanks! This fixed the failure (and others). Bootstrapped on s390x - no
> regressions:

Thanks for testing.  Also now bootstrapped & regression-tested on
x86_64-linux-gnu.  OK to install?  OK for 4.8 once 4.8.1 is released?

Thanks,
Richard
Richard Biener May 22, 2013, 9:11 a.m. UTC | #3 
On Wed, May 22, 2013 at 11:02 AM, Richard Sandiford
<rsandifo@linux.vnet.ibm.com> wrote:
> Andreas Krebbel <krebbel@linux.vnet.ibm.com> writes:
>> On 21/05/13 11:26, Richard Sandiford wrote:
>>> gcc/
>>>      * recog.c (offsettable_address_addr_space_p): Fix calculation of
>>>      address mode.  Move pointer mode initialization to the same place.
>>
>> Thanks! This fixed the failure (and others). Bootstrapped on s390x - no
>> regressions:
>
> Thanks for testing.  Also now bootstrapped & regression-tested on
> x86_64-linux-gnu.  OK to install?  OK for 4.8 once 4.8.1 is released?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
diff mbox

Patch

Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2013-04-29 12:38:00.000000000 +0100
+++ gcc/recog.c	2013-05-21 10:21:24.553671961 +0100
@@ -1953,9 +1953,6 @@  offsettable_address_addr_space_p (int st
     (strictp ? strict_memory_address_addr_space_p
 	     : memory_address_addr_space_p);
   unsigned int mode_sz = GET_MODE_SIZE (mode);
-#ifdef POINTERS_EXTEND_UNSIGNED
-  enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as);
-#endif
 
   if (CONSTANT_ADDRESS_P (y))
     return 1;
@@ -1966,6 +1963,13 @@  offsettable_address_addr_space_p (int st
   if (mode_dependent_address_p (y, as))
     return 0;
 
+  enum machine_mode address_mode = GET_MODE (y);
+  if (address_mode == VOIDmode)
+    address_mode = targetm.addr_space.address_mode (as);
+#ifdef POINTERS_EXTEND_UNSIGNED
+  enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as);
+#endif
+
   /* ??? How much offset does an offsettable BLKmode reference need?
      Clearly that depends on the situation in which it's being used.
      However, the current situation in which we test 0xffffffff is
@@ -1981,7 +1985,7 @@  offsettable_address_addr_space_p (int st
       int good;
 
       y1 = *y2;
-      *y2 = plus_constant (GET_MODE (y), *y2, mode_sz - 1);
+      *y2 = plus_constant (address_mode, *y2, mode_sz - 1);
       /* Use QImode because an odd displacement may be automatically invalid
 	 for any wider mode.  But it should be valid for a single byte.  */
       good = (*addressp) (QImode, y, as);
@@ -2002,20 +2006,20 @@  offsettable_address_addr_space_p (int st
   if (GET_CODE (y) == LO_SUM
       && mode != BLKmode
       && mode_sz <= GET_MODE_ALIGNMENT (mode) / BITS_PER_UNIT)
-    z = gen_rtx_LO_SUM (GET_MODE (y), XEXP (y, 0),
-			plus_constant (GET_MODE (y), XEXP (y, 1),
+    z = gen_rtx_LO_SUM (address_mode, XEXP (y, 0),
+			plus_constant (address_mode, XEXP (y, 1),
 				       mode_sz - 1));
 #ifdef POINTERS_EXTEND_UNSIGNED
   /* Likewise for a ZERO_EXTEND from pointer_mode.  */
   else if (POINTERS_EXTEND_UNSIGNED > 0
 	   && GET_CODE (y) == ZERO_EXTEND
 	   && GET_MODE (XEXP (y, 0)) == pointer_mode)
-    z = gen_rtx_ZERO_EXTEND (GET_MODE (y),
+    z = gen_rtx_ZERO_EXTEND (address_mode,
 			     plus_constant (pointer_mode, XEXP (y, 0),
 					    mode_sz - 1));
 #endif
   else
-    z = plus_constant (GET_MODE (y), y, mode_sz - 1);
+    z = plus_constant (address_mode, y, mode_sz - 1);
 
   /* Use QImode because an odd displacement may be automatically invalid
      for any wider mode.  But it should be valid for a single byte.  */