diff mbox

patch to fix PR65648

Message ID CAD57uCcwLCJttGUoLDJcfJF16di+w5F9Hp2B78fywnpaMD32UQ@mail.gmail.com
State New
Headers show

Commit Message

Yvan Roux April 9, 2015, 11:10 a.m. UTC
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.

Comments

Yvan Roux April 13, 2015, 2:10 p.m. UTC | #1
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
Yvan Roux April 14, 2015, 8:32 a.m. UTC | #2
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.
Yvan Roux April 14, 2015, 9:14 a.m. UTC | #3
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
diff mbox

Patch

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"
+