public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
@ 2011-08-07 20:35 H.J. Lu
  2011-08-11 13:53 ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2011-08-07 20:35 UTC (permalink / raw)
  To: gcc-patches

Hi,

We transform

ptr_extend:DI (plus:SI (FOO:SI) (const_int Y)))

to

(plus:DI (ptr_extend:DI (FOO:SI)) (const_int Y))

since this is how Pmode != ptr_mode is supported even if the resulting
address may overflow/underflow.   It is also true for x32 which has
zero_extend instead of ptr_extend.  I have tried different approaches
to avoid transforming

(zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))

to

(plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))

without success.  This patch relaxes the condition to check
POINTERS_EXTEND_UNSIGNED != 0 instead if POINTERS_EXTEND_UNSIGNED < 0
to cover both ptr_extend and zero_extend. We can investigate a better
approach for ptr_extend and zero_extend later.  For now, I believe it
is the saftest way to support ptr_extend and zero_extend.

Any comments?

Thanks.


H.J.
---
gcc/

2011-08-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space): Also permute the
	conversion and addition of constant for zero-extend.

gcc/testsuite/

2011-08-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/49721
	* gfortran.dg/pr49721.f: New.

diff --git a/gcc/explow.c b/gcc/explow.c
index f8262db..ed2f621 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -384,18 +384,23 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
 
     case PLUS:
     case MULT:
-      /* For addition we can safely permute the conversion and addition
-	 operation if one operand is a constant and converting the constant
-	 does not change it or if one operand is a constant and we are
-	 using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
+      /* FIXME: For addition, we used to permute the conversion and
+       * addition operation only if one operand is a constant and
+	 converting the constant does not change it or if one operand
+	 is a constant and we are using a ptr_extend instruction
+	 (POINTERS_EXTEND_UNSIGNED < 0) even if the resulting address
+	 may overflow/underflow.  We relax the condition to include
+	 zero-extend (POINTERS_EXTEND_UNSIGNED > 0) since the other
+	 parts of the compiler depend on it.  See PR 49721.
+
 	 We can always safely permute them if we are making the address
 	 narrower.  */
       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
 	  || (GET_CODE (x) == PLUS
 	      && CONST_INT_P (XEXP (x, 1))
-	      && (XEXP (x, 1) == convert_memory_address_addr_space
-				   (to_mode, XEXP (x, 1), as)
-                 || POINTERS_EXTEND_UNSIGNED < 0)))
+	      && (POINTERS_EXTEND_UNSIGNED != 0
+		  || XEXP (x, 1) == convert_memory_address_addr_space
+		  			(to_mode, XEXP (x, 1), as))))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
 			       convert_memory_address_addr_space
 				 (to_mode, XEXP (x, 0), as),
diff --git a/gcc/testsuite/gfortran.dg/pr49721.f b/gcc/testsuite/gfortran.dg/pr49721.f
new file mode 100644
index 0000000..39e2ed7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr49721.f
@@ -0,0 +1,35 @@
+! PR middle-end/49721
+! { dg-do compile }
+! { dg-options "-O3 -funroll-loops" }
+
+      subroutine midbloc6(c,a2,a2i,q)
+      parameter (ndim2=6)
+      parameter (ndim=3)
+      dimension ri(ndim2),cr(ndim2,ndim2),xj(ndim2,ndim2),q(*)
+     @,sai(ndim2,ndim2),cm(ndim2,ndim2),w(ndim2,ndim2)
+      dimension vr(ndim2,ndim2),vi(ndim2,ndim2),s1(ndim2,ndim2),p(ndim)
+      dimension xq(6),qb(2),qc(2),ifl(6),iplane(3)
+      save
+      call eig66(cr,rr,ri,vr,vi)
+      xq(i)=asin(ri(i))/x2pi
+      i9=6
+      qb(1)=q(1)/x2pi
+        do 180 i=1,2
+          do 170 j=1,6
+  120       if(xq(j)) 130,190,140
+  130       if(qb(i)-0.5d0) 160,150,150
+  140       if(qb(i)-0.5d0) 150,150,160
+  150       continue
+            tst=abs(abs(qb(i))-abs(xq(j)))
+  160       continue
+  170     continue
+          iplane(i)=k
+  180   continue
+  190   continue
+      n1=iplane(3)
+      if(i9.eq.6) then
+        z=vr(1,n1)*vi(2,n1)-vr(2,n1)*vi(1,n1)+vr(3,n1)*vi(4,n1)-vr(4,n1)
+      endif
+      sai(6,i)=vi(i,n1)/z
+      call dacond6(a2,zero)
+      end

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-08-07 20:35 PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns H.J. Lu
@ 2011-08-11 13:53 ` H.J. Lu
  2011-08-14 20:54   ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2011-08-11 13:53 UTC (permalink / raw)
  To: GCC Patches

Hi,

This is the last patch needed for x32 support.
convert_memory_address_addr_space
is called to convert a memory address without overflow/underflow.  It
should be safe
to transform

(zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))

to

(plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))

GCC only works this way.  Any comments?

Thanks.

H.J.
----
On Sun, Aug 7, 2011 at 1:08 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> We transform
>
> ptr_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>
> to
>
> (plus:DI (ptr_extend:DI (FOO:SI)) (const_int Y))
>
> since this is how Pmode != ptr_mode is supported even if the resulting
> address may overflow/underflow.   It is also true for x32 which has
> zero_extend instead of ptr_extend.  I have tried different approaches
> to avoid transforming
>
> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>
> to
>
> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>
> without success.  This patch relaxes the condition to check
> POINTERS_EXTEND_UNSIGNED != 0 instead if POINTERS_EXTEND_UNSIGNED < 0
> to cover both ptr_extend and zero_extend. We can investigate a better
> approach for ptr_extend and zero_extend later.  For now, I believe it
> is the saftest way to support ptr_extend and zero_extend.
>
> Any comments?
>
> Thanks.
>
>
> H.J.
> ---
> gcc/
>
> 2011-08-06  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR middle-end/49721
>        * explow.c (convert_memory_address_addr_space): Also permute the
>        conversion and addition of constant for zero-extend.
>
> gcc/testsuite/
>
> 2011-08-06  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR middle-end/49721
>        * gfortran.dg/pr49721.f: New.
>
> diff --git a/gcc/explow.c b/gcc/explow.c
> index f8262db..ed2f621 100644
> --- a/gcc/explow.c
> +++ b/gcc/explow.c
> @@ -384,18 +384,23 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
>
>     case PLUS:
>     case MULT:
> -      /* For addition we can safely permute the conversion and addition
> -        operation if one operand is a constant and converting the constant
> -        does not change it or if one operand is a constant and we are
> -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
> +      /* FIXME: For addition, we used to permute the conversion and
> +       * addition operation only if one operand is a constant and
> +        converting the constant does not change it or if one operand
> +        is a constant and we are using a ptr_extend instruction
> +        (POINTERS_EXTEND_UNSIGNED < 0) even if the resulting address
> +        may overflow/underflow.  We relax the condition to include
> +        zero-extend (POINTERS_EXTEND_UNSIGNED > 0) since the other
> +        parts of the compiler depend on it.  See PR 49721.
> +
>         We can always safely permute them if we are making the address
>         narrower.  */
>       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
>          || (GET_CODE (x) == PLUS
>              && CONST_INT_P (XEXP (x, 1))
> -             && (XEXP (x, 1) == convert_memory_address_addr_space
> -                                  (to_mode, XEXP (x, 1), as)
> -                 || POINTERS_EXTEND_UNSIGNED < 0)))
> +             && (POINTERS_EXTEND_UNSIGNED != 0
> +                 || XEXP (x, 1) == convert_memory_address_addr_space
> +                                       (to_mode, XEXP (x, 1), as))))
>        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>                               convert_memory_address_addr_space
>                                 (to_mode, XEXP (x, 0), as),
> diff --git a/gcc/testsuite/gfortran.dg/pr49721.f b/gcc/testsuite/gfortran.dg/pr49721.f
> new file mode 100644
> index 0000000..39e2ed7
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr49721.f
> @@ -0,0 +1,35 @@
> +! PR middle-end/49721
> +! { dg-do compile }
> +! { dg-options "-O3 -funroll-loops" }
> +
> +      subroutine midbloc6(c,a2,a2i,q)
> +      parameter (ndim2=6)
> +      parameter (ndim=3)
> +      dimension ri(ndim2),cr(ndim2,ndim2),xj(ndim2,ndim2),q(*)
> +     @,sai(ndim2,ndim2),cm(ndim2,ndim2),w(ndim2,ndim2)
> +      dimension vr(ndim2,ndim2),vi(ndim2,ndim2),s1(ndim2,ndim2),p(ndim)
> +      dimension xq(6),qb(2),qc(2),ifl(6),iplane(3)
> +      save
> +      call eig66(cr,rr,ri,vr,vi)
> +      xq(i)=asin(ri(i))/x2pi
> +      i9=6
> +      qb(1)=q(1)/x2pi
> +        do 180 i=1,2
> +          do 170 j=1,6
> +  120       if(xq(j)) 130,190,140
> +  130       if(qb(i)-0.5d0) 160,150,150
> +  140       if(qb(i)-0.5d0) 150,150,160
> +  150       continue
> +            tst=abs(abs(qb(i))-abs(xq(j)))
> +  160       continue
> +  170     continue
> +          iplane(i)=k
> +  180   continue
> +  190   continue
> +      n1=iplane(3)
> +      if(i9.eq.6) then
> +        z=vr(1,n1)*vi(2,n1)-vr(2,n1)*vi(1,n1)+vr(3,n1)*vi(4,n1)-vr(4,n1)
> +      endif
> +      sai(6,i)=vi(i,n1)/z
> +      call dacond6(a2,zero)
> +      end
>



-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-08-11 13:53 ` H.J. Lu
@ 2011-08-14 20:54   ` H.J. Lu
  2011-08-19 23:12     ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2011-08-14 20:54 UTC (permalink / raw)
  To: GCC Patches

Hi,

This patch is needed for x32 and only affects x32.  Any comments/objections
to apply this to finish x32 support?

Thanks.


H.J.
----
On Thu, Aug 11, 2011 at 6:25 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Hi,
>
> This is the last patch needed for x32 support.
> convert_memory_address_addr_space
> is called to convert a memory address without overflow/underflow.  It
> should be safe
> to transform
>
> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>
> to
>
> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>
> GCC only works this way.  Any comments?
>
> Thanks.
>
> H.J.
> ----
> On Sun, Aug 7, 2011 at 1:08 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> Hi,
>>
>> We transform
>>
>> ptr_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>
>> to
>>
>> (plus:DI (ptr_extend:DI (FOO:SI)) (const_int Y))
>>
>> since this is how Pmode != ptr_mode is supported even if the resulting
>> address may overflow/underflow.   It is also true for x32 which has
>> zero_extend instead of ptr_extend.  I have tried different approaches
>> to avoid transforming
>>
>> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>
>> to
>>
>> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>>
>> without success.  This patch relaxes the condition to check
>> POINTERS_EXTEND_UNSIGNED != 0 instead if POINTERS_EXTEND_UNSIGNED < 0
>> to cover both ptr_extend and zero_extend. We can investigate a better
>> approach for ptr_extend and zero_extend later.  For now, I believe it
>> is the saftest way to support ptr_extend and zero_extend.
>>
>> Any comments?
>>
>> Thanks.
>>
>>
>> H.J.
>> ---
>> gcc/
>>
>> 2011-08-06  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>        PR middle-end/49721
>>        * explow.c (convert_memory_address_addr_space): Also permute the
>>        conversion and addition of constant for zero-extend.
>>
>> gcc/testsuite/
>>
>> 2011-08-06  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>        PR middle-end/49721
>>        * gfortran.dg/pr49721.f: New.
>>
>> diff --git a/gcc/explow.c b/gcc/explow.c
>> index f8262db..ed2f621 100644
>> --- a/gcc/explow.c
>> +++ b/gcc/explow.c
>> @@ -384,18 +384,23 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
>>
>>     case PLUS:
>>     case MULT:
>> -      /* For addition we can safely permute the conversion and addition
>> -        operation if one operand is a constant and converting the constant
>> -        does not change it or if one operand is a constant and we are
>> -        using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
>> +      /* FIXME: For addition, we used to permute the conversion and
>> +       * addition operation only if one operand is a constant and
>> +        converting the constant does not change it or if one operand
>> +        is a constant and we are using a ptr_extend instruction
>> +        (POINTERS_EXTEND_UNSIGNED < 0) even if the resulting address
>> +        may overflow/underflow.  We relax the condition to include
>> +        zero-extend (POINTERS_EXTEND_UNSIGNED > 0) since the other
>> +        parts of the compiler depend on it.  See PR 49721.
>> +
>>         We can always safely permute them if we are making the address
>>         narrower.  */
>>       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
>>          || (GET_CODE (x) == PLUS
>>              && CONST_INT_P (XEXP (x, 1))
>> -             && (XEXP (x, 1) == convert_memory_address_addr_space
>> -                                  (to_mode, XEXP (x, 1), as)
>> -                 || POINTERS_EXTEND_UNSIGNED < 0)))
>> +             && (POINTERS_EXTEND_UNSIGNED != 0
>> +                 || XEXP (x, 1) == convert_memory_address_addr_space
>> +                                       (to_mode, XEXP (x, 1), as))))
>>        return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
>>                               convert_memory_address_addr_space
>>                                 (to_mode, XEXP (x, 0), as),
>> diff --git a/gcc/testsuite/gfortran.dg/pr49721.f b/gcc/testsuite/gfortran.dg/pr49721.f
>> new file mode 100644
>> index 0000000..39e2ed7
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/pr49721.f
>> @@ -0,0 +1,35 @@
>> +! PR middle-end/49721
>> +! { dg-do compile }
>> +! { dg-options "-O3 -funroll-loops" }
>> +
>> +      subroutine midbloc6(c,a2,a2i,q)
>> +      parameter (ndim2=6)
>> +      parameter (ndim=3)
>> +      dimension ri(ndim2),cr(ndim2,ndim2),xj(ndim2,ndim2),q(*)
>> +     @,sai(ndim2,ndim2),cm(ndim2,ndim2),w(ndim2,ndim2)
>> +      dimension vr(ndim2,ndim2),vi(ndim2,ndim2),s1(ndim2,ndim2),p(ndim)
>> +      dimension xq(6),qb(2),qc(2),ifl(6),iplane(3)
>> +      save
>> +      call eig66(cr,rr,ri,vr,vi)
>> +      xq(i)=asin(ri(i))/x2pi
>> +      i9=6
>> +      qb(1)=q(1)/x2pi
>> +        do 180 i=1,2
>> +          do 170 j=1,6
>> +  120       if(xq(j)) 130,190,140
>> +  130       if(qb(i)-0.5d0) 160,150,150
>> +  140       if(qb(i)-0.5d0) 150,150,160
>> +  150       continue
>> +            tst=abs(abs(qb(i))-abs(xq(j)))
>> +  160       continue
>> +  170     continue
>> +          iplane(i)=k
>> +  180   continue
>> +  190   continue
>> +      n1=iplane(3)
>> +      if(i9.eq.6) then
>> +        z=vr(1,n1)*vi(2,n1)-vr(2,n1)*vi(1,n1)+vr(3,n1)*vi(4,n1)-vr(4,n1)
>> +      endif
>> +      sai(6,i)=vi(i,n1)/z
>> +      call dacond6(a2,zero)
>> +      end
>>
>
>
>
> --
> H.J.
>



-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-08-14 20:54   ` H.J. Lu
@ 2011-08-19 23:12     ` H.J. Lu
  2011-08-26 14:11       ` Richard Sandiford
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2011-08-19 23:12 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 2550 bytes --]

On Sun, Aug 14, 2011 at 9:22 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Hi,
>
> This patch is needed for x32 and only affects x32.  Any comments/objections
> to apply this to finish x32 support?
>
> Thanks.
>
>
> H.J.
> ----
> On Thu, Aug 11, 2011 at 6:25 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Hi,
>>
>> This is the last patch needed for x32 support.
>> convert_memory_address_addr_space
>> is called to convert a memory address without overflow/underflow.  It
>> should be safe
>> to transform
>>
>> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>
>> to
>>
>> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>>
>> GCC only works this way.  Any comments?
>>
>> Thanks.
>>
>> H.J.
>> ----
>> On Sun, Aug 7, 2011 at 1:08 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>> Hi,
>>>
>>> We transform
>>>
>>> ptr_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>>
>>> to
>>>
>>> (plus:DI (ptr_extend:DI (FOO:SI)) (const_int Y))
>>>
>>> since this is how Pmode != ptr_mode is supported even if the resulting
>>> address may overflow/underflow.   It is also true for x32 which has
>>> zero_extend instead of ptr_extend.  I have tried different approaches
>>> to avoid transforming
>>>
>>> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>>
>>> to
>>>
>>> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>>>
>>> without success.  This patch relaxes the condition to check
>>> POINTERS_EXTEND_UNSIGNED != 0 instead if POINTERS_EXTEND_UNSIGNED < 0
>>> to cover both ptr_extend and zero_extend. We can investigate a better
>>> approach for ptr_extend and zero_extend later.  For now, I believe it
>>> is the saftest way to support ptr_extend and zero_extend.
>>>
>>> Any comments?
>>>
>>> Thanks.
>>>
>>>
>>> H.J.

I am checking in this patch, which only affects x32
and nothing else.  This one character change, from

POINTERS_EXTEND_UNSIGNED < 0

to

POINTERS_EXTEND_UNSIGNED != 0

creates a working x32 GCC. This isn't perfect. I have
tried many different approaches without any success.
I will revisit it if we run into any problems with x32
applications.

Thanks.

-- 
H.J.
----
gcc/

2011-08-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space): Also permute the
	conversion and addition of constant for zero-extend.

gcc/testsuite/

2011-08-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/49721
	* gfortran.dg/pr49721-1.f: New.
	* gfortran.fortran-torture/compile/pr49721-1.f: Likewise.

[-- Attachment #2: gcc-x32-pr49721-8.patch --]
[-- Type: text/x-diff, Size: 3819 bytes --]

gcc/

2011-08-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/49721
	* explow.c (convert_memory_address_addr_space): Also permute the
	conversion and addition of constant for zero-extend.

gcc/testsuite/

2011-08-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/49721
	* gfortran.dg/pr49721-1.f: New.
	* gfortran.fortran-torture/compile/pr49721-1.f: Likewise.

diff --git a/gcc/explow.c b/gcc/explow.c
index beeab44..984150e 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -384,18 +384,23 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED,
 
     case PLUS:
     case MULT:
-      /* For addition we can safely permute the conversion and addition
-	 operation if one operand is a constant and converting the constant
-	 does not change it or if one operand is a constant and we are
-	 using a ptr_extend instruction  (POINTERS_EXTEND_UNSIGNED < 0).
+      /* FIXME: For addition, we used to permute the conversion and
+	 addition operation only if one operand is a constant and
+	 converting the constant does not change it or if one operand
+	 is a constant and we are using a ptr_extend instruction
+	 (POINTERS_EXTEND_UNSIGNED < 0) even if the resulting address
+	 may overflow/underflow.  We relax the condition to include
+	 zero-extend (POINTERS_EXTEND_UNSIGNED > 0) since the other
+	 parts of the compiler depend on it.  See PR 49721.
+
 	 We can always safely permute them if we are making the address
 	 narrower.  */
       if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode)
 	  || (GET_CODE (x) == PLUS
 	      && CONST_INT_P (XEXP (x, 1))
-	      && (XEXP (x, 1) == convert_memory_address_addr_space
-				   (to_mode, XEXP (x, 1), as)
-                 || POINTERS_EXTEND_UNSIGNED < 0)))
+	      && (POINTERS_EXTEND_UNSIGNED != 0
+		  || XEXP (x, 1) == convert_memory_address_addr_space
+		  			(to_mode, XEXP (x, 1), as))))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
 			       convert_memory_address_addr_space
 				 (to_mode, XEXP (x, 0), as),
diff --git a/gcc/testsuite/gfortran.dg/pr49721-1.f b/gcc/testsuite/gfortran.dg/pr49721-1.f
new file mode 100644
index 0000000..39e2ed7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr49721-1.f
@@ -0,0 +1,35 @@
+! PR middle-end/49721
+! { dg-do compile }
+! { dg-options "-O3 -funroll-loops" }
+
+      subroutine midbloc6(c,a2,a2i,q)
+      parameter (ndim2=6)
+      parameter (ndim=3)
+      dimension ri(ndim2),cr(ndim2,ndim2),xj(ndim2,ndim2),q(*)
+     @,sai(ndim2,ndim2),cm(ndim2,ndim2),w(ndim2,ndim2)
+      dimension vr(ndim2,ndim2),vi(ndim2,ndim2),s1(ndim2,ndim2),p(ndim)
+      dimension xq(6),qb(2),qc(2),ifl(6),iplane(3)
+      save
+      call eig66(cr,rr,ri,vr,vi)
+      xq(i)=asin(ri(i))/x2pi
+      i9=6
+      qb(1)=q(1)/x2pi
+        do 180 i=1,2
+          do 170 j=1,6
+  120       if(xq(j)) 130,190,140
+  130       if(qb(i)-0.5d0) 160,150,150
+  140       if(qb(i)-0.5d0) 150,150,160
+  150       continue
+            tst=abs(abs(qb(i))-abs(xq(j)))
+  160       continue
+  170     continue
+          iplane(i)=k
+  180   continue
+  190   continue
+      n1=iplane(3)
+      if(i9.eq.6) then
+        z=vr(1,n1)*vi(2,n1)-vr(2,n1)*vi(1,n1)+vr(3,n1)*vi(4,n1)-vr(4,n1)
+      endif
+      sai(6,i)=vi(i,n1)/z
+      call dacond6(a2,zero)
+      end
diff --git a/gcc/testsuite/gfortran.fortran-torture/compile/pr49721-1.f b/gcc/testsuite/gfortran.fortran-torture/compile/pr49721-1.f
new file mode 100644
index 0000000..9ddd068
--- /dev/null
+++ b/gcc/testsuite/gfortran.fortran-torture/compile/pr49721-1.f
@@ -0,0 +1,9 @@
+       PARAMETER( LM=7 )
+      PARAMETER( NM=2+2**LM, NV=NM**3 )
+      PARAMETER( NR = (8*(NM**3+NM**2+5*NM-23+7*LM))/7 )
+	COMMON /X/ U, V, R, A
+      REAL*8 U(NR),V(NV),R(NR),A(0:3)
+      DO 20 IT=1,NIT
+        CALL RESID(U,V,R,N,A)
+ 20   CONTINUE
+      END

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-08-19 23:12     ` H.J. Lu
@ 2011-08-26 14:11       ` Richard Sandiford
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Sandiford @ 2011-08-26 14:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Sun, Aug 14, 2011 at 9:22 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Hi,
>>
>> This patch is needed for x32 and only affects x32.  Any comments/objections
>> to apply this to finish x32 support?
>>
>> Thanks.
>>
>>
>> H.J.
>> ----
>> On Thu, Aug 11, 2011 at 6:25 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> Hi,
>>>
>>> This is the last patch needed for x32 support.
>>> convert_memory_address_addr_space
>>> is called to convert a memory address without overflow/underflow.  It
>>> should be safe
>>> to transform
>>>
>>> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>>
>>> to
>>>
>>> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>>>
>>> GCC only works this way.  Any comments?
>>>
>>> Thanks.
>>>
>>> H.J.
>>> ----
>>> On Sun, Aug 7, 2011 at 1:08 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>> Hi,
>>>>
>>>> We transform
>>>>
>>>> ptr_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>>>
>>>> to
>>>>
>>>> (plus:DI (ptr_extend:DI (FOO:SI)) (const_int Y))
>>>>
>>>> since this is how Pmode != ptr_mode is supported even if the resulting
>>>> address may overflow/underflow.   It is also true for x32 which has
>>>> zero_extend instead of ptr_extend.  I have tried different approaches
>>>> to avoid transforming
>>>>
>>>> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>>>
>>>> to
>>>>
>>>> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>>>>
>>>> without success.  This patch relaxes the condition to check
>>>> POINTERS_EXTEND_UNSIGNED != 0 instead if POINTERS_EXTEND_UNSIGNED < 0
>>>> to cover both ptr_extend and zero_extend. We can investigate a better
>>>> approach for ptr_extend and zero_extend later.  For now, I believe it
>>>> is the saftest way to support ptr_extend and zero_extend.
>>>>
>>>> Any comments?
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> H.J.
>
> I am checking in this patch, which only affects x32
> and nothing else.  This one character change, from
>
> POINTERS_EXTEND_UNSIGNED < 0
>
> to
>
> POINTERS_EXTEND_UNSIGNED != 0
>
> creates a working x32 GCC. This isn't perfect. I have
> tried many different approaches without any success.
> I will revisit it if we run into any problems with x32
> applications.

Sorry, I know it's frustrating when things don't get reviewed,
but I strongly object to a nonobvious patch like this being applied
without approval.

(And for the record, I can't approve it. :-))

Richard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-07-28 19:10     ` Uros Bizjak
@ 2011-07-28 19:22       ` H.J. Lu
  0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2011-07-28 19:22 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Paolo Bonzini, gcc-patches

On Thu, Jul 28, 2011 at 11:49 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Jul 28, 2011 at 8:32 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>>>>>>> >  convert_memory_address_addr_space has a special PLUS/MULT case for
>>>>>>>> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
>>>>>>>> >  for all Pmode != ptr_mode cases. ?OK for trunk?
>>>>>>>> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>>>>>>> >
>>>>>>>> >  ? ? ? ?PR middle-end/47727
>>>>>>>> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
>>>>>>>> >  ? ? ? ?conversion and addition if one operand is a constant.
>>>>>>>>
>>>>>>>> Do we still need this patch? With recent target changes the testcase
>>>>>>>> from PR can be compiled without problems with a gcc from an unpatched
>>>>>>>> trunk.
>>>>>>>
>>>>>>> Given the communication difficulties, I hope not...
>>>>>>>
>>>>>>> Paolo
>>>>>>>
>>>>>>
>>>>>> Here is the updated patch.  OK for trunk?
>>>>>
>>>>> Did you see the question two levels up the thread you are replying to?
>>>>>
>>>>
>>>> The patch is for
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>>>>
>>>> I changed the thread subject.
>>>
>>> Please add testcase to see the patch in action.
>>>
>>
>> I haven't found a testcase yet.  The problem was discovered in
>> this thread:
>>
>> http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01065.html
>
> This was before x32 could handle SImode addresses. With recent x86
> target work, this is no more true, and SImode and DImode addresses are
> first-class citizens as far as x32 backend is concerned. Please note
> that original testcase (that this whole patch is all about) now
> compiles without problems. Also, middle end is shared with at least
> two ptr_mode != Pmode targets, and they all work well. So, to see what
> makes x32 special, we need a testcase that breaks _WITHOUT_ your
> proposed patch. Without testcase, nobody can analyze your approach and
> tell if the approach is the right one, if this is in fact target
> problem, or indeed a middle-end problem.
>

There are 2 issues

1. rtl_hooks.gen_lowpart_no_emit vs gen_lowpart. simplify-rtx.c shouldn't
generate any new insns.
2. convert_memory_address_addr_space shouldn't permute conversion and
addition.


-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-07-28 19:00   ` H.J. Lu
@ 2011-07-28 19:10     ` Uros Bizjak
  2011-07-28 19:22       ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Uros Bizjak @ 2011-07-28 19:10 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, gcc-patches

On Thu, Jul 28, 2011 at 8:32 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>>>>>> >  convert_memory_address_addr_space has a special PLUS/MULT case for
>>>>>>> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
>>>>>>> >  for all Pmode != ptr_mode cases. ?OK for trunk?
>>>>>>> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>>>>>> >
>>>>>>> >  ? ? ? ?PR middle-end/47727
>>>>>>> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
>>>>>>> >  ? ? ? ?conversion and addition if one operand is a constant.
>>>>>>>
>>>>>>> Do we still need this patch? With recent target changes the testcase
>>>>>>> from PR can be compiled without problems with a gcc from an unpatched
>>>>>>> trunk.
>>>>>>
>>>>>> Given the communication difficulties, I hope not...
>>>>>>
>>>>>> Paolo
>>>>>>
>>>>>
>>>>> Here is the updated patch.  OK for trunk?
>>>>
>>>> Did you see the question two levels up the thread you are replying to?
>>>>
>>>
>>> The patch is for
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>>>
>>> I changed the thread subject.
>>
>> Please add testcase to see the patch in action.
>>
>
> I haven't found a testcase yet.  The problem was discovered in
> this thread:
>
> http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01065.html

This was before x32 could handle SImode addresses. With recent x86
target work, this is no more true, and SImode and DImode addresses are
first-class citizens as far as x32 backend is concerned. Please note
that original testcase (that this whole patch is all about) now
compiles without problems. Also, middle end is shared with at least
two ptr_mode != Pmode targets, and they all work well. So, to see what
makes x32 special, we need a testcase that breaks _WITHOUT_ your
proposed patch. Without testcase, nobody can analyze your approach and
tell if the approach is the right one, if this is in fact target
problem, or indeed a middle-end problem.

And there is no point to flood the mainling-list with patches.

Uros.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-07-28 18:49 ` Uros Bizjak
@ 2011-07-28 19:00   ` H.J. Lu
  2011-07-28 19:10     ` Uros Bizjak
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2011-07-28 19:00 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Paolo Bonzini, gcc-patches

On Thu, Jul 28, 2011 at 11:23 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Jul 28, 2011 at 8:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>>>>> >  convert_memory_address_addr_space has a special PLUS/MULT case for
>>>>>> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
>>>>>> >  for all Pmode != ptr_mode cases. ?OK for trunk?
>>>>>> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>>>>> >
>>>>>> >  ? ? ? ?PR middle-end/47727
>>>>>> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
>>>>>> >  ? ? ? ?conversion and addition if one operand is a constant.
>>>>>>
>>>>>> Do we still need this patch? With recent target changes the testcase
>>>>>> from PR can be compiled without problems with a gcc from an unpatched
>>>>>> trunk.
>>>>>
>>>>> Given the communication difficulties, I hope not...
>>>>>
>>>>> Paolo
>>>>>
>>>>
>>>> Here is the updated patch.  OK for trunk?
>>>
>>> Did you see the question two levels up the thread you are replying to?
>>>
>>
>> The patch is for
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>>
>> I changed the thread subject.
>
> Please add testcase to see the patch in action.
>

I haven't found a testcase yet.  The problem was discovered in
this thread:

http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01065.html


-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
  2011-07-28 18:30 H.J. Lu
@ 2011-07-28 18:49 ` Uros Bizjak
  2011-07-28 19:00   ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Uros Bizjak @ 2011-07-28 18:49 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, gcc-patches

On Thu, Jul 28, 2011 at 8:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>>>> >  convert_memory_address_addr_space has a special PLUS/MULT case for
>>>>> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
>>>>> >  for all Pmode != ptr_mode cases. ?OK for trunk?
>>>>> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>>>> >
>>>>> >  ? ? ? ?PR middle-end/47727
>>>>> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
>>>>> >  ? ? ? ?conversion and addition if one operand is a constant.
>>>>>
>>>>> Do we still need this patch? With recent target changes the testcase
>>>>> from PR can be compiled without problems with a gcc from an unpatched
>>>>> trunk.
>>>>
>>>> Given the communication difficulties, I hope not...
>>>>
>>>> Paolo
>>>>
>>>
>>> Here is the updated patch.  OK for trunk?
>>
>> Did you see the question two levels up the thread you are replying to?
>>
>
> The patch is for
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>
> I changed the thread subject.

Please add testcase to see the patch in action.

Uros.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
@ 2011-07-28 18:30 H.J. Lu
  2011-07-28 18:49 ` Uros Bizjak
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2011-07-28 18:30 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Paolo Bonzini, gcc-patches

On Thu, Jul 28, 2011 at 11:05 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Jul 28, 2011 at 7:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>>> >  convert_memory_address_addr_space has a special PLUS/MULT case for
>>>> >  POINTERS_EXTEND_UNSIGNED<  0. ?It turns out that it is also needed
>>>> >  for all Pmode != ptr_mode cases. ?OK for trunk?
>>>> >  2011-06-11 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>>> >
>>>> >  ? ? ? ?PR middle-end/47727
>>>> >  ? ? ? ?* explow.c (convert_memory_address_addr_space): Permute the
>>>> >  ? ? ? ?conversion and addition if one operand is a constant.
>>>>
>>>> Do we still need this patch? With recent target changes the testcase
>>>> from PR can be compiled without problems with a gcc from an unpatched
>>>> trunk.
>>>
>>> Given the communication difficulties, I hope not...
>>>
>>> Paolo
>>>
>>
>> Here is the updated patch.  OK for trunk?
>
> Did you see the question two levels up the thread you are replying to?
>

The patch is for

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721

I changed the thread subject.

-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-08-26 13:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-07 20:35 PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns H.J. Lu
2011-08-11 13:53 ` H.J. Lu
2011-08-14 20:54   ` H.J. Lu
2011-08-19 23:12     ` H.J. Lu
2011-08-26 14:11       ` Richard Sandiford
  -- strict thread matches above, loose matches on Subject: below --
2011-07-28 18:30 H.J. Lu
2011-07-28 18:49 ` Uros Bizjak
2011-07-28 19:00   ` H.J. Lu
2011-07-28 19:10     ` Uros Bizjak
2011-07-28 19:22       ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).