Patchwork [i386,4.6] Backport movd*_internal_rex64 fix from trunk and [4.6/4.7] add testcase

login
register
mail settings
Submitter Uros Bizjak
Date Dec. 7, 2011, 10:12 p.m.
Message ID <CAFULd4ZQo_KV-Rp81661D74Br1d_Nyx0p4gCYeWRM3qFGU0D-Q@mail.gmail.com>
Download mbox | patch
Permalink /patch/130047/
State New
Headers show

Comments

Uros Bizjak - Dec. 7, 2011, 10:12 p.m.
Hello!

> An issue turned up in our internal 4.6 based testing that has been
> fixed on trunk. This patch backports the fix to 4.6. I also have a
> small test case that I will add to both 4.6 and 4.7.
>
> Bootstrapped and checked with x86_64-unknown-linux-gnu.
>
> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>
> 	Backport from mainline:
>
> 	2011-08-05  Uros Bizjak  <ubizjak@gmail.com>
>
> 	* config/i386/i386.md (*movdi_internal_rex64): Use "!o" constraint
> 	instead of "!m" for operand 0, alternative 4.
> 	(*movdf_internal_rex64): Ditto for operand 0, alernative 6.
>
> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>
> 	* gcc.target/i386/movdi-rex64.c: New.


You don't need #include for compile tests, just use:

--cut here--
/* { dg-do compile } */
/* { dg-options "-fPIE" } */

char *strcpy (char *dest, const char *src);

static __thread char buffer[25];

const char
* error_message (void)
{
  strcpy (buffer, "Unknown code ");
  return 0;
}
--cut here--

Also this can be compiled everywhere, not just linux.

OK with the testcase above.

Thanks,
Uros.
Teresa Johnson - Dec. 7, 2011, 10:32 p.m.
On Wed, Dec 7, 2011 at 2:12 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
>> An issue turned up in our internal 4.6 based testing that has been
>> fixed on trunk. This patch backports the fix to 4.6. I also have a
>> small test case that I will add to both 4.6 and 4.7.
>>
>> Bootstrapped and checked with x86_64-unknown-linux-gnu.
>>
>> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>>
>>       Backport from mainline:
>>
>>       2011-08-05  Uros Bizjak  <ubizjak@gmail.com>
>>
>>       * config/i386/i386.md (*movdi_internal_rex64): Use "!o" constraint
>>       instead of "!m" for operand 0, alternative 4.
>>       (*movdf_internal_rex64): Ditto for operand 0, alernative 6.
>>
>> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>>
>>       * gcc.target/i386/movdi-rex64.c: New.
>
> Index: testsuite/gcc.target/i386/movdi-rex64.c
> ===================================================================
> --- testsuite/gcc.target/i386/movdi-rex64.c     (revision 0)
> +++ testsuite/gcc.target/i386/movdi-rex64.c     (revision 0)
> @@ -0,0 +1,11 @@
> +/* { dg-do compile { target *-*-linux* } } */
> +/* { dg-options "-fPIE -Wwrite-strings" } */
> +
> +#include <string.h>
> +static __thread char buffer[25];
> +const char * error_message (void)
> +{
> +oops:
> +    strcpy (buffer, "Unknown code ");
> +    return 0;
> +}
>
> You don't need #include for compile tests, just use:
>
> --cut here--
> /* { dg-do compile } */
> /* { dg-options "-fPIE" } */
>
> char *strcpy (char *dest, const char *src);
>
> static __thread char buffer[25];
>
> const char
> * error_message (void)
> {
>  strcpy (buffer, "Unknown code ");
>  return 0;
> }
> --cut here--
>
> Also this can be compiled everywhere, not just linux.

Ok, I will change the testcase to replace the include and retest.

Regarding the linux target restriction, though, I was concerned about
the -fPIE option used for the test case. I noticed that in 4.7 there
is a "pie" effective target keyword (check_effective_target_pie in
testsuite/lib/target-supports.exp). However, that does not yet exist
in 4.6, so rather than backport that as well I added the linux
restriction. I see the same restriction in the other tests that use
-fpie in gcc.target/i386 (pr39013-[12].c). What do you think?

Thanks!
Teresa

>
> OK with the testcase above.
>
> Thanks,
> Uros.
Uros Bizjak - Dec. 8, 2011, 6:55 a.m.
On Wed, Dec 7, 2011 at 11:32 PM, Teresa Johnson <tejohnson@google.com> wrote:

>>> An issue turned up in our internal 4.6 based testing that has been
>>> fixed on trunk. This patch backports the fix to 4.6. I also have a
>>> small test case that I will add to both 4.6 and 4.7.
>>>
>>> Bootstrapped and checked with x86_64-unknown-linux-gnu.
>>>
>>> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>>>
>>>       Backport from mainline:
>>>
>>>       2011-08-05  Uros Bizjak  <ubizjak@gmail.com>
>>>
>>>       * config/i386/i386.md (*movdi_internal_rex64): Use "!o" constraint
>>>       instead of "!m" for operand 0, alternative 4.
>>>       (*movdf_internal_rex64): Ditto for operand 0, alernative 6.
>>>
>>> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>>>
>>>       * gcc.target/i386/movdi-rex64.c: New.
>>
>> Index: testsuite/gcc.target/i386/movdi-rex64.c
>> ===================================================================
>> --- testsuite/gcc.target/i386/movdi-rex64.c     (revision 0)
>> +++ testsuite/gcc.target/i386/movdi-rex64.c     (revision 0)
>> @@ -0,0 +1,11 @@
>> +/* { dg-do compile { target *-*-linux* } } */
>> +/* { dg-options "-fPIE -Wwrite-strings" } */
>> +
>> +#include <string.h>
>> +static __thread char buffer[25];
>> +const char * error_message (void)
>> +{
>> +oops:
>> +    strcpy (buffer, "Unknown code ");
>> +    return 0;
>> +}
>>
>> You don't need #include for compile tests, just use:
>>
>> --cut here--
>> /* { dg-do compile } */
>> /* { dg-options "-fPIE" } */
>>
>> char *strcpy (char *dest, const char *src);
>>
>> static __thread char buffer[25];
>>
>> const char
>> * error_message (void)
>> {
>>  strcpy (buffer, "Unknown code ");
>>  return 0;
>> }
>> --cut here--
>>
>> Also this can be compiled everywhere, not just linux.
>
> Ok, I will change the testcase to replace the include and retest.
>
> Regarding the linux target restriction, though, I was concerned about
> the -fPIE option used for the test case. I noticed that in 4.7 there
> is a "pie" effective target keyword (check_effective_target_pie in
> testsuite/lib/target-supports.exp). However, that does not yet exist
> in 4.6, so rather than backport that as well I added the linux
> restriction. I see the same restriction in the other tests that use
> -fpie in gcc.target/i386 (pr39013-[12].c). What do you think?

Ah, I see. Then pleasee add back linux target selctor for 4.6 and add
fpie effective target check for 4.7.

Thanks,
Uros.
Teresa Johnson - Dec. 8, 2011, 2:23 p.m.
Sounds good. I will fix the 4.7 version to use the fpie target
selector. The other thing I noticed is that you had removed the
-Wwrite-strings option, but I needed to keep that as it was the
combination of -fPIE and -Wwrite-strings that triggered the bug.

Thanks,
Teresa

On Wed, Dec 7, 2011 at 10:55 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Dec 7, 2011 at 11:32 PM, Teresa Johnson <tejohnson@google.com> wrote:
>
>>>> An issue turned up in our internal 4.6 based testing that has been
>>>> fixed on trunk. This patch backports the fix to 4.6. I also have a
>>>> small test case that I will add to both 4.6 and 4.7.
>>>>
>>>> Bootstrapped and checked with x86_64-unknown-linux-gnu.
>>>>
>>>> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>>>>
>>>>       Backport from mainline:
>>>>
>>>>       2011-08-05  Uros Bizjak  <ubizjak@gmail.com>
>>>>
>>>>       * config/i386/i386.md (*movdi_internal_rex64): Use "!o" constraint
>>>>       instead of "!m" for operand 0, alternative 4.
>>>>       (*movdf_internal_rex64): Ditto for operand 0, alernative 6.
>>>>
>>>> 2011-12-07  Teresa Johnson  <tejohnson@google.com>
>>>>
>>>>       * gcc.target/i386/movdi-rex64.c: New.
>>>
>>> Index: testsuite/gcc.target/i386/movdi-rex64.c
>>> ===================================================================
>>> --- testsuite/gcc.target/i386/movdi-rex64.c     (revision 0)
>>> +++ testsuite/gcc.target/i386/movdi-rex64.c     (revision 0)
>>> @@ -0,0 +1,11 @@
>>> +/* { dg-do compile { target *-*-linux* } } */
>>> +/* { dg-options "-fPIE -Wwrite-strings" } */
>>> +
>>> +#include <string.h>
>>> +static __thread char buffer[25];
>>> +const char * error_message (void)
>>> +{
>>> +oops:
>>> +    strcpy (buffer, "Unknown code ");
>>> +    return 0;
>>> +}
>>>
>>> You don't need #include for compile tests, just use:
>>>
>>> --cut here--
>>> /* { dg-do compile } */
>>> /* { dg-options "-fPIE" } */
>>>
>>> char *strcpy (char *dest, const char *src);
>>>
>>> static __thread char buffer[25];
>>>
>>> const char
>>> * error_message (void)
>>> {
>>>  strcpy (buffer, "Unknown code ");
>>>  return 0;
>>> }
>>> --cut here--
>>>
>>> Also this can be compiled everywhere, not just linux.
>>
>> Ok, I will change the testcase to replace the include and retest.
>>
>> Regarding the linux target restriction, though, I was concerned about
>> the -fPIE option used for the test case. I noticed that in 4.7 there
>> is a "pie" effective target keyword (check_effective_target_pie in
>> testsuite/lib/target-supports.exp). However, that does not yet exist
>> in 4.6, so rather than backport that as well I added the linux
>> restriction. I see the same restriction in the other tests that use
>> -fpie in gcc.target/i386 (pr39013-[12].c). What do you think?
>
> Ah, I see. Then pleasee add back linux target selctor for 4.6 and add
> fpie effective target check for 4.7.
>
> Thanks,
> Uros.
Uros Bizjak - Dec. 8, 2011, 4:23 p.m.
On Thu, Dec 8, 2011 at 3:23 PM, Teresa Johnson <tejohnson@google.com> wrote:
> Sounds good. I will fix the 4.7 version to use the fpie target
> selector. The other thing I noticed is that you had removed the
> -Wwrite-strings option, but I needed to keep that as it was the
> combination of -fPIE and -Wwrite-strings that triggered the bug.

No, the testacase ICEes also without -Wwrite-strings on unpatched gcc.
It is a warning option.

Uros.
Teresa Johnson - Dec. 8, 2011, 4:30 p.m.
Interesting - with our internal compiler version it only triggered
with the 2 options combined, not sure why. But I just verified that on
the gcc 4.6 branch it only needed the -fPIE option. Sorry about that,
I will remove the -W option from the test case.

Teresa

On Thu, Dec 8, 2011 at 8:23 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Dec 8, 2011 at 3:23 PM, Teresa Johnson <tejohnson@google.com> wrote:
>> Sounds good. I will fix the 4.7 version to use the fpie target
>> selector. The other thing I noticed is that you had removed the
>> -Wwrite-strings option, but I needed to keep that as it was the
>> combination of -fPIE and -Wwrite-strings that triggered the bug.
>
> No, the testacase ICEes also without -Wwrite-strings on unpatched gcc.
> It is a warning option.
>
> Uros.
Uros Bizjak - Dec. 8, 2011, 4:43 p.m.
On Thu, Dec 8, 2011 at 5:30 PM, Teresa Johnson <tejohnson@google.com> wrote:
> Interesting - with our internal compiler version it only triggered
> with the 2 options combined, not sure why. But I just verified that on
> the gcc 4.6 branch it only needed the -fPIE option. Sorry about that,
> I will remove the -W option from the test case.

Please also remove unused label, gcc ICEs without it.

Uros.
Teresa Johnson - Dec. 8, 2011, 5:02 p.m.
Verified and changed.

Thanks!
Teresa

On Thu, Dec 8, 2011 at 8:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, Dec 8, 2011 at 5:30 PM, Teresa Johnson <tejohnson@google.com> wrote:
>> Interesting - with our internal compiler version it only triggered
>> with the 2 options combined, not sure why. But I just verified that on
>> the gcc 4.6 branch it only needed the -fPIE option. Sorry about that,
>> I will remove the -W option from the test case.
>
> Please also remove unused label, gcc ICEs without it.
>
> Uros.

Patch

Index: testsuite/gcc.target/i386/movdi-rex64.c
===================================================================
--- testsuite/gcc.target/i386/movdi-rex64.c	(revision 0)
+++ testsuite/gcc.target/i386/movdi-rex64.c	(revision 0)
@@ -0,0 +1,11 @@ 
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-fPIE -Wwrite-strings" } */
+
+#include <string.h>
+static __thread char buffer[25];
+const char * error_message (void)
+{
+oops:
+    strcpy (buffer, "Unknown code ");
+    return 0;
+}