PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

[H.J. Lu]

Hi,

I just don't see how nonzero_bits1 can assume if pointers extend unsigned
and this is an addition or subtraction to a pointer in Pmode, all the bits
bove ptr_mode are known to be zero.  We never run into it before x32
since x32 is the first such target.

This patch deletes it.  OK to install the nonzero_bits1 part for trunk?

Thanks.


H.J.
---
diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32
index 564e123..9bbb05b 100644
--- a/gcc/ChangeLog.x32
+++ b/gcc/ChangeLog.x32
@@ -1,3 +1,9 @@
+2011-06-22  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR rtl-optimization/49504
+	* rtlanal.c (nonzero_bits1): Properly handle addition or
+	subtraction a pointer in Pmode if pointers extend unsigned.
+
 2011-06-15  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR middle-end/48016
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index b52957d..e5c045d 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4134,20 +4134,6 @@ nonzero_bits1 (const_rtx x, enum machine_mode mode, const_rtx known_x,
 
 	if (result_low > 0)
 	  nonzero &= ~(((unsigned HOST_WIDE_INT) 1 << result_low) - 1);
-
-#ifdef POINTERS_EXTEND_UNSIGNED
-	/* If pointers extend unsigned and this is an addition or subtraction
-	   to a pointer in Pmode, all the bits above ptr_mode are known to be
-	   zero.  */
-	/* As we do not know which address space the pointer is refering to,
-	   we can do this only if the target does not support different pointer
-	   or address modes depending on the address space.  */
-	if (target_default_pointer_address_modes_p ()
-	    && POINTERS_EXTEND_UNSIGNED > 0 && GET_MODE (x) == Pmode
-	    && (code == PLUS || code == MINUS)
-	    && REG_P (XEXP (x, 0)) && REG_POINTER (XEXP (x, 0)))
-	  nonzero &= GET_MODE_MASK (ptr_mode);
-#endif
       }
       break;
 
diff --git a/gcc/testsuite/ChangeLog.x32 b/gcc/testsuite/ChangeLog.x32
index ab8dd76..6581a45 100644
--- a/gcc/testsuite/ChangeLog.x32
+++ b/gcc/testsuite/ChangeLog.x32
@@ -1,3 +1,8 @@
+2011-06-22  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR rtl-optimization/49504
+	* gcc.target/i386/pr49504.c: New.
+
 2011-06-15  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* gcc.dg/pr44194-1.c: Also allow x32.
diff --git a/gcc/testsuite/gcc.target/i386/pr49504.c b/gcc/testsuite/gcc.target/i386/pr49504.c
new file mode 100644
index 0000000..9128196
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr49504.c
@@ -0,0 +1,18 @@
+/* PR target/49504 */
+/* { dg-do run { target { x32 } } } */
+/* { dg-options "-O" } */
+
+unsigned long long 
+foo (const void* p, unsigned long long q)
+{
+  unsigned long long a = (((unsigned long long) ((unsigned long) p)) + q) >> 32;
+  return a;
+}
+
+int
+main ()
+{
+  if (foo ((const void*) 0x100, 0x100000000ULL) == 0)
+    __builtin_abort ();
+  return 0;
+}

Re: PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

[H.J. Lu]

On Wed, Jun 22, 2011 at 12:36:57PM -0700, H.J. Lu wrote:
> Hi,
> 
> I just don't see how nonzero_bits1 can assume if pointers extend unsigned
> and this is an addition or subtraction to a pointer in Pmode, all the bits
> bove ptr_mode are known to be zero.  We never run into it before x32
> since x32 is the first such target.
> 
> This patch deletes it.  OK to install the nonzero_bits1 part for trunk?
> 
> Thanks.
> 
> 

I checked this patch into x32 branch.

H.J.
---
commit de145b6ad18327c34009d96f1a1f0a9510023f31
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Jun 23 06:09:20 2011 -0700

    Check correct return value.

diff --git a/gcc/testsuite/ChangeLog.x32 b/gcc/testsuite/ChangeLog.x32
index 6581a45..cde8d41 100644
--- a/gcc/testsuite/ChangeLog.x32
+++ b/gcc/testsuite/ChangeLog.x32
@@ -1,3 +1,7 @@
+2011-06-23  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* gcc.target/i386/pr49504.c (main): Check correct return value.
+
 2011-06-22  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR rtl-optimization/49504
diff --git a/gcc/testsuite/gcc.target/i386/pr49504.c b/gcc/testsuite/gcc.target/i386/pr49504.c
index 9128196..503e6c2 100644
--- a/gcc/testsuite/gcc.target/i386/pr49504.c
+++ b/gcc/testsuite/gcc.target/i386/pr49504.c
@@ -12,7 +12,7 @@ foo (const void* p, unsigned long long q)
 int
 main ()
 {
-  if (foo ((const void*) 0x100, 0x100000000ULL) == 0)
+  if (foo (foo, 0x100000000ULL) != 0x1)
     __builtin_abort ();
   return 0;
 }

Re: PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

[Eric Botcazou]

> I just don't see how nonzero_bits1 can assume if pointers extend unsigned
> and this is an addition or subtraction to a pointer in Pmode, all the bits
> bove ptr_mode are known to be zero.  We never run into it before x32
> since x32 is the first such target.

I agree that this is overly optimistical, but this was done on purpose:
  http://gcc.gnu.org/ml/gcc-patches/2001-02/msg00316.html

Could you evaluate the pessimization that the patch introduces (if any) for the 
generated code on x32?  If there is none or it is negligible, the patch is OK 
if completed by the removal of the equivalent code in num_sign_bit_copies1.
If it isn't negligible, we may need to be clever and devise something else.

-- 
Eric Botcazou

Re: PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

[H.J. Lu]

On Fri, Jun 24, 2011 at 1:58 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> I just don't see how nonzero_bits1 can assume if pointers extend unsigned
>> and this is an addition or subtraction to a pointer in Pmode, all the bits
>> bove ptr_mode are known to be zero.  We never run into it before x32
>> since x32 is the first such target.
>
> I agree that this is overly optimistical, but this was done on purpose:
>  http://gcc.gnu.org/ml/gcc-patches/2001-02/msg00316.html
>
> Could you evaluate the pessimization that the patch introduces (if any) for the
> generated code on x32?  If there is none or it is negligible, the patch is OK
> if completed by the removal of the equivalent code in num_sign_bit_copies1.
> If it isn't negligible, we may need to be clever and devise something else.
>

I compared x32 glibc built with the old x32 gcc against x32 glibc built with
this patch and

http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00913.html

reverted, the new glibc is little smaller:

New:

[hjl@gnu-33 build-x86_64-linux]$ size libc.so
   text	   data	    bss	    dec	    hex	filename
1537648	  10076	  12944	1560668	 17d05c	libc.so
[hjl@gnu-33 build-x86_64-linux]$

Old:

[hjl@gnu-33 build-x86_64-linux.old]$ size libc.so
   text	   data	    bss	    dec	    hex	filename
1538968	  10076	  12944	1561988	 17d584	libc.so
[hjl@gnu-33 build-x86_64-linux.old]$

I looked at assembly codes.  The new one is better.
I will check it in.

Thanks.

-- 
H.J.

Re: PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

[Eric Botcazou]

> I compared x32 glibc built with the old x32 gcc against x32 glibc built
> with this patch and
>
> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00913.html
>
> reverted, the new glibc is little smaller:
>
> New:
>
> [hjl@gnu-33 build-x86_64-linux]$ size libc.so
>    text	   data	    bss	    dec	    hex	filename
> 1537648	  10076	  12944	1560668	 17d05c	libc.so
> [hjl@gnu-33 build-x86_64-linux]$
>
> Old:
>
> [hjl@gnu-33 build-x86_64-linux.old]$ size libc.so
>    text	   data	    bss	    dec	    hex	filename
> 1538968	  10076	  12944	1561988	 17d584	libc.so
> [hjl@gnu-33 build-x86_64-linux.old]$
>
> I looked at assembly codes.  The new one is better.
> I will check it in.

OK, but remove the equivalent code in num_sign_bit_copies1 then, otherwise 
someone in a couple of years from now will adapt it to nonzero_bits1 and we 
will be back to square one.

-- 
Eric Botcazou

Re: PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

[H.J. Lu]

On Fri, Jun 24, 2011 at 7:07 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> I compared x32 glibc built with the old x32 gcc against x32 glibc built
>> with this patch and
>>
>> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00913.html
>>
>> reverted, the new glibc is little smaller:
>>
>> New:
>>
>> [hjl@gnu-33 build-x86_64-linux]$ size libc.so
>>    text          data     bss     dec     hex filename
>> 1537648         10076   12944 1560668  17d05c libc.so
>> [hjl@gnu-33 build-x86_64-linux]$
>>
>> Old:
>>
>> [hjl@gnu-33 build-x86_64-linux.old]$ size libc.so
>>    text          data     bss     dec     hex filename
>> 1538968         10076   12944 1561988  17d584 libc.so
>> [hjl@gnu-33 build-x86_64-linux.old]$
>>
>> I looked at assembly codes.  The new one is better.
>> I will check it in.
>
> OK, but remove the equivalent code in num_sign_bit_copies1 then, otherwise
> someone in a couple of years from now will adapt it to nonzero_bits1 and we
> will be back to square one.
>

I am testing this patch on x32 branch.  I will compare glibc binaries before and
after.

Thanks.

-- 
H.J.
---
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index e5c045d..0be6504 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4605,21 +4605,6 @@ num_sign_bit_copies1 (const_rtx x, enum
machine_mode mode, const_rtx known_x,
 					 known_x, known_mode, known_ret);
       result = MAX (1, MIN (num0, num1) - 1);

-#ifdef POINTERS_EXTEND_UNSIGNED
-      /* If pointers extend signed and this is an addition or subtraction
-	 to a pointer in Pmode, all the bits above ptr_mode are known to be
-	 sign bit copies.  */
-      /* As we do not know which address space the pointer is refering to,
-	 we can do this only if the target does not support different pointer
-	 or address modes depending on the address space.  */
-      if (target_default_pointer_address_modes_p ()
-	  && ! POINTERS_EXTEND_UNSIGNED && GET_MODE (x) == Pmode
-	  && (code == PLUS || code == MINUS)
-	  && REG_P (XEXP (x, 0)) && REG_POINTER (XEXP (x, 0)))
-	result = MAX ((int) (GET_MODE_BITSIZE (Pmode)
-			     - GET_MODE_BITSIZE (ptr_mode) + 1),
-		      result);
-#endif
       return result;

     case MULT:

Re: PATCH: PR rtl-optimization/49504: Invalid optimization for Pmode != ptr_mode

[H.J. Lu]

On Fri, Jun 24, 2011 at 7:17 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jun 24, 2011 at 7:07 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> I compared x32 glibc built with the old x32 gcc against x32 glibc built
>>> with this patch and
>>>
>>> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00913.html
>>>
>>> reverted, the new glibc is little smaller:
>>>
>>> New:
>>>
>>> [hjl@gnu-33 build-x86_64-linux]$ size libc.so
>>>    text          data     bss     dec     hex filename
>>> 1537648         10076   12944 1560668  17d05c libc.so
>>> [hjl@gnu-33 build-x86_64-linux]$
>>>
>>> Old:
>>>
>>> [hjl@gnu-33 build-x86_64-linux.old]$ size libc.so
>>>    text          data     bss     dec     hex filename
>>> 1538968         10076   12944 1561988  17d584 libc.so
>>> [hjl@gnu-33 build-x86_64-linux.old]$
>>>
>>> I looked at assembly codes.  The new one is better.
>>> I will check it in.
>>
>> OK, but remove the equivalent code in num_sign_bit_copies1 then, otherwise
>> someone in a couple of years from now will adapt it to nonzero_bits1 and we
>> will be back to square one.
>>
>
> I am testing this patch on x32 branch.  I will compare glibc binaries before and
> after.
>

There are no code differences in glibc.  I will check it in.

Thanks.

H.J.
> --
> H.J.
> ---
> diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
> index e5c045d..0be6504 100644
> --- a/gcc/rtlanal.c
> +++ b/gcc/rtlanal.c
> @@ -4605,21 +4605,6 @@ num_sign_bit_copies1 (const_rtx x, enum
> machine_mode mode, const_rtx known_x,
>                                         known_x, known_mode, known_ret);
>       result = MAX (1, MIN (num0, num1) - 1);
>
> -#ifdef POINTERS_EXTEND_UNSIGNED
> -      /* If pointers extend signed and this is an addition or subtraction
> -        to a pointer in Pmode, all the bits above ptr_mode are known to be
> -        sign bit copies.  */
> -      /* As we do not know which address space the pointer is refering to,
> -        we can do this only if the target does not support different pointer
> -        or address modes depending on the address space.  */
> -      if (target_default_pointer_address_modes_p ()
> -         && ! POINTERS_EXTEND_UNSIGNED && GET_MODE (x) == Pmode
> -         && (code == PLUS || code == MINUS)
> -         && REG_P (XEXP (x, 0)) && REG_POINTER (XEXP (x, 0)))
> -       result = MAX ((int) (GET_MODE_BITSIZE (Pmode)
> -                            - GET_MODE_BITSIZE (ptr_mode) + 1),
> -                     result);
> -#endif
>       return result;
>
>     case MULT:
>



-- 
H.J.