public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH: PR target/52146: [x32] - Wrong code to access addresses 0x80000000 to 0xFFFFFFFF
@ 2012-02-10 17:42 H.J. Lu
  2012-02-10 17:44 ` H.J. Lu
  0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2012-02-10 17:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: Uros Bizjak

Hi,

Since constant address in x32 is signed extended to 64bit, negative
displacement without base nor index is out of range.  OK for trunk?

Thanks.


H.J.
---
gcc/

2012-02-10  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/52146
	* config/i386/i386.c (ix86_legitimate_address_p): Disallow
	negative constant address for x32.

gcc/testsuite/

2012-02-10  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/52146
	* gcc.target/i386/pr52146.c: New.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 009dd53..0bb94a7 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12107,6 +12107,15 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
 		   || !ix86_legitimate_constant_p (Pmode, disp)))
 	/* Displacement is not constant.  */
 	return false;
+      else if (TARGET_X32
+	       && !base
+	       && !index
+	       && CONST_INT_P (disp)
+	       && INTVAL (disp) < 0)
+	/* Since constant address in x32 is signed extended to 64bit,
+	   negative displacement without base nor index is out of
+	   range.  */
+	return false;
       else if (TARGET_64BIT
 	       && !x86_64_immediate_operand (disp, VOIDmode))
 	/* Displacement is out of range.  */
diff --git a/gcc/testsuite/gcc.target/i386/pr52146.c b/gcc/testsuite/gcc.target/i386/pr52146.c
new file mode 100644
index 0000000..68bdeff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr52146.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { { i?86-*-linux* x86_64-*-linux* } && { ! { ia32 } } } } } */
+/* { dg-options "-O2 -mx32" } */
+
+void test1() {
+  int* apic_tpr_addr = (int *)0xfee00080;
+  *apic_tpr_addr += 4;
+}
+void test2() {
+  volatile int* apic_tpr_addr = (int *)0xfee00080;
+  *apic_tpr_addr = 0;
+}
+void test3() {
+  volatile int* apic_tpr_addr = (int *)0x7fffffff;
+  *apic_tpr_addr = 0;
+}
+
+/* { dg-final { scan-assembler-not "-18874240" } } */

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

* Re: PATCH: PR target/52146: [x32] - Wrong code to access addresses 0x80000000 to 0xFFFFFFFF
  2012-02-10 17:42 PATCH: PR target/52146: [x32] - Wrong code to access addresses 0x80000000 to 0xFFFFFFFF H.J. Lu
@ 2012-02-10 17:44 ` H.J. Lu
  2012-02-10 18:01   ` Uros Bizjak
  0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2012-02-10 17:44 UTC (permalink / raw)
  To: gcc-patches, Uros Bizjak

On Fri, Feb 10, 2012 at 09:25:06AM -0800, H.J. Lu wrote:
> Hi,
> 
> Since constant address in x32 is signed extended to 64bit, negative
> displacement without base nor index is out of range.  OK for trunk?
> 

Here is a different patch.

H.J.
---
gcc/

2012-02-10  Uros Bizjak  <ubizjak@gmail.com>

	PR target/52146
	* config/i386/i386.c (ix86_legitimate_address_p): Disallow
	negative constant address for x32.

gcc/testsuite/

2012-02-10  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/52146
	* gcc.target/i386/pr52146.c: New.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 009dd53..8f4e72e 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -11932,6 +11932,13 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
   rtx base, index, disp;
   HOST_WIDE_INT scale;
 
+  /* Since constant address in x32 is signed extended to 64bit,
+     we have to prevent addresses from 0x80000000 to 0xffffffff.  */
+  if (TARGET_X32
+      && CONST_INT_P (addr)
+      && val_signbit_known_set_p (SImode, INTVAL (addr)))
+    return false;
+
   if (ix86_decompose_address (addr, &parts) <= 0)
     /* Decomposition failed.  */
     return false;
diff --git a/gcc/testsuite/gcc.target/i386/pr52146.c b/gcc/testsuite/gcc.target/i386/pr52146.c
new file mode 100644
index 0000000..68bdeff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr52146.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target { { i?86-*-linux* x86_64-*-linux* } && { ! { ia32 } } } } } */
+/* { dg-options "-O2 -mx32" } */
+
+void test1() {
+  int* apic_tpr_addr = (int *)0xfee00080;
+  *apic_tpr_addr += 4;
+}
+void test2() {
+  volatile int* apic_tpr_addr = (int *)0xfee00080;
+  *apic_tpr_addr = 0;
+}
+void test3() {
+  volatile int* apic_tpr_addr = (int *)0x7fffffff;
+  *apic_tpr_addr = 0;
+}
+
+/* { dg-final { scan-assembler-not "-18874240" } } */

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

* Re: PATCH: PR target/52146: [x32] - Wrong code to access addresses 0x80000000 to 0xFFFFFFFF
  2012-02-10 17:44 ` H.J. Lu
@ 2012-02-10 18:01   ` Uros Bizjak
  2012-02-10 18:25     ` H.J. Lu
  0 siblings, 1 reply; 4+ messages in thread
From: Uros Bizjak @ 2012-02-10 18:01 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches

On Fri, Feb 10, 2012 at 6:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:

>> Since constant address in x32 is signed extended to 64bit, negative
>> displacement without base nor index is out of range.  OK for trunk?
>>
>
> Here is a different patch.
>
> H.J.
> ---
> gcc/
>
> 2012-02-10  Uros Bizjak  <ubizjak@gmail.com>
>
>        PR target/52146
>        * config/i386/i386.c (ix86_legitimate_address_p): Disallow
>        negative constant address for x32.
>
> gcc/testsuite/
>
> 2012-02-10  H.J. Lu  <hongjiu.lu@intel.com>
>
>        PR target/52146
>        * gcc.target/i386/pr52146.c: New.
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 009dd53..8f4e72e 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -11932,6 +11932,13 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
>   rtx base, index, disp;
>   HOST_WIDE_INT scale;
>
> +  /* Since constant address in x32 is signed extended to 64bit,
> +     we have to prevent addresses from 0x80000000 to 0xffffffff.  */
> +  if (TARGET_X32
> +      && CONST_INT_P (addr)
> +      && val_signbit_known_set_p (SImode, INTVAL (addr)))

As said in the PR, val_signbit_known_set_p is a bit overkill. Please
use INTVAL (addr) < 0, it works as well.

> +++ b/gcc/testsuite/gcc.target/i386/pr52146.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile { target { { i?86-*-linux* x86_64-*-linux* } && { ! { ia32 } } } } } */

we _are_ in x86 directory, so:

/* { dg-do compile { target { ! { ia32 } } } }  */

> +/* { dg-options "-O2 -mx32" } */
> +
> +void test1() {
> +  int* apic_tpr_addr = (int *)0xfee00080;
> +  *apic_tpr_addr += 4;
> +}
> +void test2() {
> +  volatile int* apic_tpr_addr = (int *)0xfee00080;
> +  *apic_tpr_addr = 0;

No need for volatile.

> +}
> +void test3() {
> +  volatile int* apic_tpr_addr = (int *)0x7fffffff;
> +  *apic_tpr_addr = 0;
> +}

test2 is enough. No need to test what worked OK.

> +
> +/* { dg-final { scan-assembler-not "-18874240" } } */

Please also reformat the test to GNU coding standards.

Patch is OK for 4.7 and 4.6 after bootstrap and regression test on x32 target.

Thanks,
Uros.

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

* Re: PATCH: PR target/52146: [x32] - Wrong code to access addresses 0x80000000 to 0xFFFFFFFF
  2012-02-10 18:01   ` Uros Bizjak
@ 2012-02-10 18:25     ` H.J. Lu
  0 siblings, 0 replies; 4+ messages in thread
From: H.J. Lu @ 2012-02-10 18:25 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches

On Fri, Feb 10, 2012 at 9:55 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Feb 10, 2012 at 6:42 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>
>>> Since constant address in x32 is signed extended to 64bit, negative
>>> displacement without base nor index is out of range.  OK for trunk?
>>>
>>
>> Here is a different patch.
>>
>> H.J.
>> ---
>> gcc/
>>
>> 2012-02-10  Uros Bizjak  <ubizjak@gmail.com>
>>
>>        PR target/52146
>>        * config/i386/i386.c (ix86_legitimate_address_p): Disallow
>>        negative constant address for x32.
>>
>> gcc/testsuite/
>>
>> 2012-02-10  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>        PR target/52146
>>        * gcc.target/i386/pr52146.c: New.
>>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index 009dd53..8f4e72e 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -11932,6 +11932,13 @@ ix86_legitimate_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
>>   rtx base, index, disp;
>>   HOST_WIDE_INT scale;
>>
>> +  /* Since constant address in x32 is signed extended to 64bit,
>> +     we have to prevent addresses from 0x80000000 to 0xffffffff.  */
>> +  if (TARGET_X32
>> +      && CONST_INT_P (addr)
>> +      && val_signbit_known_set_p (SImode, INTVAL (addr)))
>
> As said in the PR, val_signbit_known_set_p is a bit overkill. Please
> use INTVAL (addr) < 0, it works as well.
>
>> +++ b/gcc/testsuite/gcc.target/i386/pr52146.c
>> @@ -0,0 +1,17 @@
>> +/* { dg-do compile { target { { i?86-*-linux* x86_64-*-linux* } && { ! { ia32 } } } } } */
>
> we _are_ in x86 directory, so:
>
> /* { dg-do compile { target { ! { ia32 } } } }  */
>
>> +/* { dg-options "-O2 -mx32" } */
>> +
>> +void test1() {
>> +  int* apic_tpr_addr = (int *)0xfee00080;
>> +  *apic_tpr_addr += 4;
>> +}
>> +void test2() {
>> +  volatile int* apic_tpr_addr = (int *)0xfee00080;
>> +  *apic_tpr_addr = 0;
>
> No need for volatile.
>
>> +}
>> +void test3() {
>> +  volatile int* apic_tpr_addr = (int *)0x7fffffff;
>> +  *apic_tpr_addr = 0;
>> +}
>
> test2 is enough. No need to test what worked OK.
>
>> +
>> +/* { dg-final { scan-assembler-not "-18874240" } } */
>
> Please also reformat the test to GNU coding standards.
>
> Patch is OK for 4.7 and 4.6 after bootstrap and regression test on x32 target.

I checked it into 4.7 and will backport to my x32 4.6 branch.

Thanks.

-- 
H.J.

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

end of thread, other threads:[~2012-02-10 18:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-10 17:42 PATCH: PR target/52146: [x32] - Wrong code to access addresses 0x80000000 to 0xFFFFFFFF H.J. Lu
2012-02-10 17:44 ` H.J. Lu
2012-02-10 18:01   ` Uros Bizjak
2012-02-10 18:25     ` 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).