patch to fix PR65648

patch to fix PR65648

[Vladimir Makarov]

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

   The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65648

   The patch was bootstrapped and tested on x86-64.

   I am lost to produce a test for the PR which can work on all arm 
sub-targets and have no sub-target hardware to test it.
   Therefore the patch does not contain the test.  It would be nice if 
somebody knowing arm well add a test for the PR.

   Committed as rev. 221901.

2015-04-07  Vladimir Makarov  <vmakarov@redhat.com>

         PR target/65678
         * lra-remat.c (do_remat): Process input and non-input insn
         registers separately.



[-- Attachment #2: pr65648.patch --]
[-- Type: text/x-patch, Size: 1648 bytes --]

Index: lra-remat.c
===================================================================
--- lra-remat.c	(revision 221867)
+++ lra-remat.c	(working copy)
@@ -1234,22 +1234,25 @@ do_remat (void)
 		for (i = 0; i < nregs; i++)
 		  CLEAR_HARD_REG_BIT (live_hard_regs, hard_regno + i);
 	      }
-	    else if (reg->type != OP_IN
-		     && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL)
+	  /* Process also hard regs (e.g. CC register) which are part
+	     of insn definition.  */
+	  for (reg = static_id->hard_regs; reg != NULL; reg = reg->next)
+	    if (reg->type == OP_IN
+		&& find_regno_note (insn, REG_DEAD, reg->regno) != NULL)
+	      CLEAR_HARD_REG_BIT (live_hard_regs, reg->regno);
+	  /* Inputs have been processed, now process outputs.  */
+	  for (reg = id->regs; reg != NULL; reg = reg->next)
+	    if (reg->type != OP_IN
+		&& find_regno_note (insn, REG_UNUSED, reg->regno) == NULL)
 	      {
 		if ((hard_regno = get_hard_regs (reg, nregs)) < 0)
 		  continue;
 		for (i = 0; i < nregs; i++)
 		  SET_HARD_REG_BIT (live_hard_regs, hard_regno + i);
 	      }
-	  /* Process also hard regs (e.g. CC register) which are part
-	     of insn definition.  */
 	  for (reg = static_id->hard_regs; reg != NULL; reg = reg->next)
-	    if (reg->type == OP_IN
-		&& find_regno_note (insn, REG_DEAD, reg->regno) != NULL)
-	      CLEAR_HARD_REG_BIT (live_hard_regs, reg->regno);
-	    else if (reg->type != OP_IN
-		     && find_regno_note (insn, REG_UNUSED, reg->regno) == NULL)
+	    if (reg->type != OP_IN
+		&& find_regno_note (insn, REG_UNUSED, reg->regno) == NULL)
 	      SET_HARD_REG_BIT (live_hard_regs, reg->regno);
 	}
     }

Re: patch to fix PR65648

[Jakub Jelinek]

On Tue, Apr 07, 2015 at 11:01:59AM -0400, Vladimir Makarov wrote:
>   The following patch fixes
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65648
> 
>   The patch was bootstrapped and tested on x86-64.
> 
>   I am lost to produce a test for the PR which can work on all arm
> sub-targets and have no sub-target hardware to test it.
>   Therefore the patch does not contain the test.  It would be nice if
> somebody knowing arm well add a test for the PR.
> 
>   Committed as rev. 221901.
> 
> 2015-04-07  Vladimir Makarov  <vmakarov@redhat.com>
> 
>         PR target/65678

PR target/65648
instead?

	Jakub

Re: patch to fix PR65648

[Jakub Jelinek]

On Tue, Apr 07, 2015 at 11:01:59AM -0400, Vladimir Makarov wrote:
> 2015-04-07  Vladimir Makarov  <vmakarov@redhat.com>
> 
>         PR target/65678
>         * lra-remat.c (do_remat): Process input and non-input insn
>         registers separately.

Don't have a quick access to arm box right now (without waiting for it to be
installed etc.), but using a cross-compiler I can at least reproduce
that your patch changes also:

/* PR target/65648 */

int a = 0, *b = 0, c = 0;
static int d = 0;
short e = 1;
static long long f = 0;
long long *i = &f;
unsigned char j = 0;

__attribute__((noinline, noclone)) void
foo (int x, int *y)
{
  asm volatile ("" : : "r" (x), "r" (y) : "memory");
}

__attribute__((noinline, noclone)) void
bar (const char *x, long long y)
{
  asm volatile ("" : : "r" (x), "r" (&y) : "memory");
  if (y != 0)
    __builtin_abort ();
}

int
main ()
{
  int k = 0;
  b = &k;
  j = (!a) - (c <= e);
  *i = j;
  foo (a, &k);
  bar ("", f);
  return 0;
}

so, if anybody can confirm this testcase aborts with -march=armv6-m -mthumb -Os
before Vlad's patch and doesn't abort afterwards, perhaps just sticking
that into gcc.c-torture/execute/pr65648.c would be sufficient.
Or, if people don't regularly test with -march=armv6-m -mthumb combination,
perhaps put it into gcc.c-torture/execute/pr65648.c as is and
add another gcc.target/arm/pr65648.c testcase that will #include this one,
and use the right dg-options / dg-skip-if or dg-require-effective-target etc.
for it to trigger.  As the testcase uses uninitialized r3 in the wrong case,
I wonder if the usual _start initializes r3 to some value that triggers the abort
without the fix; if not, perhaps it needs to be in another function and the caller
should somehow attempt to set l3 somehow (pass arguments to another function etc.)
to a value that will trigger the abort.

	Jakub

Re: patch to fix PR65648

[Yvan Roux]

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

Hi Jakub,

On 7 April 2015 at 17:51, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Apr 07, 2015 at 11:01:59AM -0400, Vladimir Makarov wrote:
>> 2015-04-07  Vladimir Makarov  <vmakarov@redhat.com>
>>
>>         PR target/65678
>>         * lra-remat.c (do_remat): Process input and non-input insn
>>         registers separately.
>
> Don't have a quick access to arm box right now (without waiting for it to be
> installed etc.), but using a cross-compiler I can at least reproduce
> that your patch changes also:
>
> /* PR target/65648 */
>
> int a = 0, *b = 0, c = 0;
> static int d = 0;
> short e = 1;
> static long long f = 0;
> long long *i = &f;
> unsigned char j = 0;
>
> __attribute__((noinline, noclone)) void
> foo (int x, int *y)
> {
>   asm volatile ("" : : "r" (x), "r" (y) : "memory");
> }
>
> __attribute__((noinline, noclone)) void
> bar (const char *x, long long y)
> {
>   asm volatile ("" : : "r" (x), "r" (&y) : "memory");
>   if (y != 0)
>     __builtin_abort ();
> }
>
> int
> main ()
> {
>   int k = 0;
>   b = &k;
>   j = (!a) - (c <= e);
>   *i = j;
>   foo (a, &k);
>   bar ("", f);
>   return 0;
> }
>
> so, if anybody can confirm this testcase aborts with -march=armv6-m -mthumb -Os
> before Vlad's patch and doesn't abort afterwards, perhaps just sticking
> that into gcc.c-torture/execute/pr65648.c would be sufficient.
> Or, if people don't regularly test with -march=armv6-m -mthumb combination,
> perhaps put it into gcc.c-torture/execute/pr65648.c as is and
> add another gcc.target/arm/pr65648.c testcase that will #include this one,
> and use the right dg-options / dg-skip-if or dg-require-effective-target etc.
> for it to trigger.  As the testcase uses uninitialized r3 in the wrong case,
> I wonder if the usual _start initializes r3 to some value that triggers the abort
> without the fix; if not, perhaps it needs to be in another function and the caller
> should somehow attempt to set l3 somehow (pass arguments to another function etc.)
> to a value that will trigger the abort.

validation is ongoing, but here is my attempt to add this testcase,
does it look correct (it's the first time I use that kind of include
in testsuite)

Cheers,
Yvan

[-- Attachment #2: pr65648-testcase.diff --]
[-- Type: text/plain, Size: 1374 bytes --]

diff --git a/gcc/testsuite/gcc.c-torture/execute/pr65648.c b/gcc/testsuite/gcc.c-torture/execute/pr65648.c
new file mode 100644
index 0000000..88a2fc9
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr65648.c
@@ -0,0 +1,34 @@
+/* PR target/65648 */
+
+int a = 0, *b = 0, c = 0;
+static int d = 0;
+short e = 1;
+static long long f = 0;
+long long *i = &f;
+unsigned char j = 0;
+
+__attribute__((noinline, noclone)) void
+foo (int x, int *y)
+{
+  asm volatile ("" : : "r" (x), "r" (y) : "memory");
+}
+
+__attribute__((noinline, noclone)) void
+bar (const char *x, long long y)
+{
+  asm volatile ("" : : "r" (x), "r" (&y) : "memory");
+  if (y != 0)
+    __builtin_abort ();
+}
+
+int
+main ()
+{
+  int k = 0;
+  b = &k;
+  j = (!a) - (c <= e);
+  *i = j;
+  foo (a, &k);
+  bar ("", f);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/pr65648.c b/gcc/testsuite/gcc.target/arm/pr65648.c
new file mode 100644
index 0000000..215c6a9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr65648.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6-m" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" } { "" } } */
+/* { dg-options "-mthumb -Os" } */
+/* { dg-add-options arm_arch_v6m } */
+
+#include "../../gcc.c-torture/execute/pr65648.c"
+

Re: patch to fix PR65648

[Jakub Jelinek]

On Tue, Apr 07, 2015 at 09:28:51PM +0200, Yvan Roux wrote:
> validation is ongoing, but here is my attempt to add this testcase,
> does it look correct (it's the first time I use that kind of include
> in testsuite)

The intent is that we have a testcase for all targets at various
optimization levels, plus one with specific options for the particular
target.
If you get at least one FAIL with this patch with Vlad's patch reverted and
no FAILs with that patch, the patch is ok for trunk with the obvious
ChangeLog entry.

Thanks.

	Jakub

Re: patch to fix PR65648

[Yvan Roux]

On 7 April 2015 at 21:33, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Apr 07, 2015 at 09:28:51PM +0200, Yvan Roux wrote:
>> validation is ongoing, but here is my attempt to add this testcase,
>> does it look correct (it's the first time I use that kind of include
>> in testsuite)
>
> The intent is that we have a testcase for all targets at various
> optimization levels, plus one with specific options for the particular
> target.
> If you get at least one FAIL with this patch with Vlad's patch reverted and
> no FAILs with that patch, the patch is ok for trunk with the obvious
> ChangeLog entry.

Ok I understand.

Thanks

> Thanks.
>
>         Jakub

Re: patch to fix PR65648

[Yvan Roux]

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

Hi

On 7 April 2015 at 22:02, Yvan Roux <yvan.roux@linaro.org> wrote:
> On 7 April 2015 at 21:33, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Tue, Apr 07, 2015 at 09:28:51PM +0200, Yvan Roux wrote:
>>> validation is ongoing, but here is my attempt to add this testcase,
>>> does it look correct (it's the first time I use that kind of include
>>> in testsuite)
>>
>> The intent is that we have a testcase for all targets at various
>> optimization levels, plus one with specific options for the particular
>> target.
>> If you get at least one FAIL with this patch with Vlad's patch reverted and
>> no FAILs with that patch, the patch is ok for trunk with the obvious
>> ChangeLog entry.

As this testcase needs to be executed, we can have some conflict at
link time, depending on how the libs were compiled. Here is what I've
for the moment, let me know if it's ok and/or if you have suggestion
on how to improve it.

- armv6 doesn't support the hard-float ABI in Thumb mode, I disable
the testcase with this directive, but not sure it's the best way:
{ dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } {
"*" } { "" } }ot

- The original problem was reported on armv6-m arch. but is not
related to the M profile, if we stick to armv6-m the link will fail on
compiler which default to the -A profile.  As my guess is that -A
profile is more widely tested, I changed the -march flag to armv6. Do
you think it's ok ?

With the attached patch there is only 1 FAIL before Vlad's patch on
the execution of the test and they all PASS when testing
arm-linux-gnueabi, and the target test is UNSUPPORTED when test
arm-linux-gnueabihf.

Cheers,
Yvan

2105-04-09  Yvan Roux  <yvan.roux@linaro.org>

        PR target/65648
        * gcc.c-torture/execute/pr65648.c: New test.
        * gcc.target/arm/pr65648.c: New test.

[-- Attachment #2: test.diff --]
[-- Type: text/plain, Size: 1474 bytes --]

diff --git a/gcc/testsuite/gcc.c-torture/execute/pr65648.c b/gcc/testsuite/gcc.c-torture/execute/pr65648.c
new file mode 100644
index 0000000..88a2fc9
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr65648.c
@@ -0,0 +1,34 @@
+/* PR target/65648 */
+
+int a = 0, *b = 0, c = 0;
+static int d = 0;
+short e = 1;
+static long long f = 0;
+long long *i = &f;
+unsigned char j = 0;
+
+__attribute__((noinline, noclone)) void
+foo (int x, int *y)
+{
+  asm volatile ("" : : "r" (x), "r" (y) : "memory");
+}
+
+__attribute__((noinline, noclone)) void
+bar (const char *x, long long y)
+{
+  asm volatile ("" : : "r" (x), "r" (&y) : "memory");
+  if (y != 0)
+    __builtin_abort ();
+}
+
+int
+main ()
+{
+  int k = 0;
+  b = &k;
+  j = (!a) - (c <= e);
+  *i = j;
+  foo (a, &k);
+  bar ("", f);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/arm/pr65648.c b/gcc/testsuite/gcc.target/arm/pr65648.c
new file mode 100644
index 0000000..e075546
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr65648.c
@@ -0,0 +1,9 @@
+/* { dg-do run } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" } { "" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } { "*" } { "" } } */
+/* { dg-options "-mthumb -Os -mfloat-abi=soft" } */
+/* { dg-add-options arm_arch_v6 } */
+
+#include "../../gcc.c-torture/execute/pr65648.c"
+

Re: patch to fix PR65648

[Jakub Jelinek]

On Thu, Apr 09, 2015 at 01:10:41PM +0200, Yvan Roux wrote:
> 2105-04-09  Yvan Roux  <yvan.roux@linaro.org>
> 
>         PR target/65648
>         * gcc.c-torture/execute/pr65648.c: New test.

This part is definitely ok for trunk.

>         * gcc.target/arm/pr65648.c: New test.

This part should better be reviewed by some ARM maintainer, I'm really lost
in all the incompatible ARM options.

	Jakub

Re: patch to fix PR65648

[Kyrill Tkachov]

On 09/04/15 12:10, Yvan Roux wrote:
> diff --git a/gcc/testsuite/gcc.target/arm/pr65648.c b/gcc/testsuite/gcc.target/arm/pr65648.c
> new file mode 100644
> index 0000000..e075546
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr65648.c
> @@ -0,0 +1,9 @@
> +/* { dg-do run } */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6" } } */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" } { "" } } */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } { "*" } { "" } } */
> +/* { dg-options "-mthumb -Os -mfloat-abi=soft" } */
> +/* { dg-add-options arm_arch_v6 } */
> +
> +#include "../../gcc.c-torture/execute/pr65648.c"
> +
Hi Yvan,

These are always tough to get right.
How about:
/* { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } { "*" } { "" } } */
/* { dg-options "-Os -mthumb -mfloat-abi=soft" } */
/* { dg-add-options arm_arch_v6 } */
/* { dg-require-effective-target arm_arch_v6_ok } */
?

I think  the dg-skip-if will avoid the error when testing arm-none-linux-gnueabihf:
  "error: ./pr65648.exe uses VFP register arguments, /tmp/ccXpRQ41.o does not"

The dg-require-effective-target should remove the need for the first dg-skip-if in your options.
I don't think it's worth skipping the test when the user explicitly asks for -marm. It won't test the
behaviour of the bug but then again, the user overrode the options, so presumably knows best.

Is there any case where this fails?

Kyrill

Re: patch to fix PR65648

[Yvan Roux]

On 13 April 2015 at 15:42, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>
> On 09/04/15 12:10, Yvan Roux wrote:
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/pr65648.c
>> b/gcc/testsuite/gcc.target/arm/pr65648.c
>> new file mode 100644
>> index 0000000..e075546
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/pr65648.c
>> @@ -0,0 +1,9 @@
>> +/* { dg-do run } */
>> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
>> "-march=*" } { "-march=armv6" } } */
>> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm"
>> } { "" } } */
>> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } {
>> "*" } { "" } } */
>> +/* { dg-options "-mthumb -Os -mfloat-abi=soft" } */
>> +/* { dg-add-options arm_arch_v6 } */
>> +
>> +#include "../../gcc.c-torture/execute/pr65648.c"
>> +
>
> Hi Yvan,
>
> These are always tough to get right.
> How about:
> /* { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } { "*" }
> { "" } } */
> /* { dg-options "-Os -mthumb -mfloat-abi=soft" } */
> /* { dg-add-options arm_arch_v6 } */
> /* { dg-require-effective-target arm_arch_v6_ok } */
> ?
>
> I think  the dg-skip-if will avoid the error when testing
> arm-none-linux-gnueabihf:
>  "error: ./pr65648.exe uses VFP register arguments, /tmp/ccXpRQ41.o does
> not"
>
> The dg-require-effective-target should remove the need for the first
> dg-skip-if in your options.
> I don't think it's worth skipping the test when the user explicitly asks for
> -marm. It won't test the
> behaviour of the bug but then again, the user overrode the options, so
> presumably knows best.

Yes it looks better to me, the -marm skipping in my patch was an
artifact of testing it for armv6-m

> Is there any case where this fails?

none that I can think of,  Is it ok to commit after I've re-tested it
(maybe once 5.1 is released) ?

Yvan

Re: patch to fix PR65648

[Kyrill Tkachov]

On 13/04/15 15:10, Yvan Roux wrote:
> On 13 April 2015 at 15:42, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> On 09/04/15 12:10, Yvan Roux wrote:
>>> diff --git a/gcc/testsuite/gcc.target/arm/pr65648.c
>>> b/gcc/testsuite/gcc.target/arm/pr65648.c
>>> new file mode 100644
>>> index 0000000..e075546
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/pr65648.c
>>> @@ -0,0 +1,9 @@
>>> +/* { dg-do run } */
>>> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
>>> "-march=*" } { "-march=armv6" } } */
>>> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm"
>>> } { "" } } */
>>> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } {
>>> "*" } { "" } } */
>>> +/* { dg-options "-mthumb -Os -mfloat-abi=soft" } */
>>> +/* { dg-add-options arm_arch_v6 } */
>>> +
>>> +#include "../../gcc.c-torture/execute/pr65648.c"
>>> +
>> Hi Yvan,
>>
>> These are always tough to get right.
>> How about:
>> /* { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } { "*" }
>> { "" } } */
>> /* { dg-options "-Os -mthumb -mfloat-abi=soft" } */
>> /* { dg-add-options arm_arch_v6 } */
>> /* { dg-require-effective-target arm_arch_v6_ok } */
>> ?
>>
>> I think  the dg-skip-if will avoid the error when testing
>> arm-none-linux-gnueabihf:
>>   "error: ./pr65648.exe uses VFP register arguments, /tmp/ccXpRQ41.o does
>> not"
>>
>> The dg-require-effective-target should remove the need for the first
>> dg-skip-if in your options.
>> I don't think it's worth skipping the test when the user explicitly asks for
>> -marm. It won't test the
>> behaviour of the bug but then again, the user overrode the options, so
>> presumably knows best.
> Yes it looks better to me, the -marm skipping in my patch was an
> artifact of testing it for armv6-m
>
>> Is there any case where this fails?
> none that I can think of,  Is it ok to commit after I've re-tested it
> (maybe once 5.1 is released) ?

Yes, the arm part is ok. I believe Jakub ok'ed the gcc.c-torture hunk.
I think it can go in now, as it is a testcase for a PR that was fixed for GCC 5.
Does it need to be committed to the release branch as well?

Thanks,
Kyrill

>
> Yvan
>

Re: patch to fix PR65648

[Jakub Jelinek]

On Mon, Apr 13, 2015 at 03:36:06PM +0100, Kyrill Tkachov wrote:
> Yes, the arm part is ok. I believe Jakub ok'ed the gcc.c-torture hunk.
> I think it can go in now, as it is a testcase for a PR that was fixed for GCC 5.
> Does it need to be committed to the release branch as well?

Yes, but it probably should wait until 5.1 is released, no reason to rush it
in immediately.

	Jakub

Re: patch to fix PR65648

[Ramana Radhakrishnan]

On Thu, Apr 9, 2015 at 12:10 PM, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi
>
> On 7 April 2015 at 22:02, Yvan Roux <yvan.roux@linaro.org> wrote:
>> On 7 April 2015 at 21:33, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Tue, Apr 07, 2015 at 09:28:51PM +0200, Yvan Roux wrote:
>>>> validation is ongoing, but here is my attempt to add this testcase,
>>>> does it look correct (it's the first time I use that kind of include
>>>> in testsuite)
>>>
>>> The intent is that we have a testcase for all targets at various
>>> optimization levels, plus one with specific options for the particular
>>> target.
>>> If you get at least one FAIL with this patch with Vlad's patch reverted and
>>> no FAILs with that patch, the patch is ok for trunk with the obvious
>>> ChangeLog entry.
>
> As this testcase needs to be executed, we can have some conflict at
> link time, depending on how the libs were compiled. Here is what I've
> for the moment, let me know if it's ok and/or if you have suggestion
> on how to improve it.
>
> - armv6 doesn't support the hard-float ABI in Thumb mode, I disable
> the testcase with this directive, but not sure it's the best way:
> { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } {
> "*" } { "" } }ot
>
> - The original problem was reported on armv6-m arch. but is not
> related to the M profile, if we stick to armv6-m the link will fail on
> compiler which default to the -A profile.  As my guess is that -A
> profile is more widely tested, I changed the -march flag to armv6. Do
> you think it's ok ?

IMO there are enough folks who test M profile. I'd drop all the arch
specific options and just apply the patch.

Thanks,
Ramana

>
> With the attached patch there is only 1 FAIL before Vlad's patch on
> the execution of the test and they all PASS when testing
> arm-linux-gnueabi, and the target test is UNSUPPORTED when test
> arm-linux-gnueabihf.
>
> Cheers,
> Yvan
>
> 2105-04-09  Yvan Roux  <yvan.roux@linaro.org>
>
>         PR target/65648
>         * gcc.c-torture/execute/pr65648.c: New test.
>         * gcc.target/arm/pr65648.c: New test.

Re: patch to fix PR65648

[Yvan Roux]

On 14 April 2015 at 10:19, Ramana Radhakrishnan
<ramana.gcc@googlemail.com> wrote:
> On Thu, Apr 9, 2015 at 12:10 PM, Yvan Roux <yvan.roux@linaro.org> wrote:
>> Hi
>>
>> On 7 April 2015 at 22:02, Yvan Roux <yvan.roux@linaro.org> wrote:
>>> On 7 April 2015 at 21:33, Jakub Jelinek <jakub@redhat.com> wrote:
>>>> On Tue, Apr 07, 2015 at 09:28:51PM +0200, Yvan Roux wrote:
>>>>> validation is ongoing, but here is my attempt to add this testcase,
>>>>> does it look correct (it's the first time I use that kind of include
>>>>> in testsuite)
>>>>
>>>> The intent is that we have a testcase for all targets at various
>>>> optimization levels, plus one with specific options for the particular
>>>> target.
>>>> If you get at least one FAIL with this patch with Vlad's patch reverted and
>>>> no FAILs with that patch, the patch is ok for trunk with the obvious
>>>> ChangeLog entry.
>>
>> As this testcase needs to be executed, we can have some conflict at
>> link time, depending on how the libs were compiled. Here is what I've
>> for the moment, let me know if it's ok and/or if you have suggestion
>> on how to improve it.
>>
>> - armv6 doesn't support the hard-float ABI in Thumb mode, I disable
>> the testcase with this directive, but not sure it's the best way:
>> { dg-skip-if "avoid conflicting multilib options" { *-*-*eabihf } {
>> "*" } { "" } }ot
>>
>> - The original problem was reported on armv6-m arch. but is not
>> related to the M profile, if we stick to armv6-m the link will fail on
>> compiler which default to the -A profile.  As my guess is that -A
>> profile is more widely tested, I changed the -march flag to armv6. Do
>> you think it's ok ?
>
> IMO there are enough folks who test M profile. I'd drop all the arch
> specific options and just apply the patch.

The issue is more related to armv6 than M profile, but if it is widely
tested as well I can just commit the torture test if it's ok for
Jakub.

Thanks,
Yvan

> Thanks,
> Ramana
>
>>
>> With the attached patch there is only 1 FAIL before Vlad's patch on
>> the execution of the test and they all PASS when testing
>> arm-linux-gnueabi, and the target test is UNSUPPORTED when test
>> arm-linux-gnueabihf.
>>
>> Cheers,
>> Yvan
>>
>> 2105-04-09  Yvan Roux  <yvan.roux@linaro.org>
>>
>>         PR target/65648
>>         * gcc.c-torture/execute/pr65648.c: New test.
>>         * gcc.target/arm/pr65648.c: New test.

Re: patch to fix PR65648

[Jakub Jelinek]

On Tue, Apr 14, 2015 at 10:32:16AM +0200, Yvan Roux wrote:
> The issue is more related to armv6 than M profile, but if it is widely
> tested as well I can just commit the torture test if it's ok for
> Jakub.

If it is tested by enough people, just the execute.exp test is ok of course.

	Jakub

Re: patch to fix PR65648

[Ramana Radhakrishnan]

On Tue, Apr 14, 2015 at 9:33 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Apr 14, 2015 at 10:32:16AM +0200, Yvan Roux wrote:
>> The issue is more related to armv6 than M profile, but if it is widely
>> tested as well I can just commit the torture test if it's ok for
>> Jakub.
>
> If it is tested by enough people, just the execute.exp test is ok of course.
>
>         Jakub

Yeah the execute.exp test is fine.

Ramana

Re: patch to fix PR65648

[Yvan Roux]

On 14 April 2015 at 10:35, Ramana Radhakrishnan
<ramana.gcc@googlemail.com> wrote:
> On Tue, Apr 14, 2015 at 9:33 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Tue, Apr 14, 2015 at 10:32:16AM +0200, Yvan Roux wrote:
>>> The issue is more related to armv6 than M profile, but if it is widely
>>> tested as well I can just commit the torture test if it's ok for
>>> Jakub.
>>
>> If it is tested by enough people, just the execute.exp test is ok of course.
>>
>>         Jakub
>
> Yeah the execute.exp test is fine.

Ok, I'll commit it this afternoon then.

Thanks
YVan

> Ramana