Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
On Thu, 15 Nov 2018 09:48:05 -0500 Paul Koning wrote: > > On Nov 14, 2018, at 5:19 PM, Jozef Lawrynowicz > > wrote: > > > > > > I'd be curious if the line I added the xfail to in c-c++-common/pr57371-2.c > > also fails for pdp11. > > > > The conversion to float might be getting optimized out whenever > > sizeof(int) < sizeof(float). > > > > Thanks, > > Jozef > > Yes, that test on pr57371-2.c also fails on pdp11. > > paul > Thanks for checking, in that case I'll go ahead an add an effective target for "int_lt_float". I'll make a note to investigate that test failure as well. The test comments: > We can not get rid of comparison in tests below because of > potential inexact exception. If I'm understanding the test correctly, then if the cast to float has been optimized out, users expecting the inexact float exception to be raised will have unexpected behaviour. Jozef
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
> On Nov 14, 2018, at 5:19 PM, Jozef Lawrynowicz
> wrote:
>
> On Wed, 14 Nov 2018 11:30:39 -0500
> Paul Koning wrote:
>
>>> On Nov 14, 2018, at 10:44 AM, Jozef Lawrynowicz
>>> wrote:
>>>
>>> Patch 1 tweaks dg directives in tests specifically for msp430. Many of
>>> these are extensions to existing target selectors in dg directives.
>>>
>>> <0001-TESTSUITE-MSP430-Tweak-dg-directives-for-msp430-elf.patch>
>>
>> For pr41779.c, you have
>>
>> +/* { dg-skip-if "int is smaller than float" { msp430-*-* } } */
>>
>> I take it that means: sizeof(int) < sizeof(float). That property also holds
>> for pdp11 and perhaps other targets. Would it make sense to introduce a new
>> effective-target flag for that check instead?
>>
>> paul
>>
>
> Paul,
>
> Yes you are correct the comment implies sizeof(int) < sizeof(float).
>
> I believe this was the only test where this property affected the test
> results, so a new effective-target flag is probably only worth adding if it
> affects at least a couple of tests.
> On the other hand, I suppose there is no harm in adding another
> check-effective-target and it at least means we'll catch failures across more
> targets.
>
> I'd be curious if the line I added the xfail to in c-c++-common/pr57371-2.c
> also fails for pdp11.
>
> The conversion to float might be getting optimized out whenever
> sizeof(int) < sizeof(float).
>
> Thanks,
> Jozef
Yes, that test on pr57371-2.c also fails on pdp11.
paul
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
On Thu, 15 Nov 2018 10:36:57 +0100 Andreas Schwab wrote: > On Nov 14 2018, Jozef Lawrynowicz wrote: > > > The timeout as set in the dejagnu configuration for msp430 > > ([dejagnu.git]/baseboards/msp430-sim.exp) is 30, which is rarely > > hit. > > I don't think it makes sense for a board file to set a smaller timeout > than the default. > > Andreas. > Hmm, yes I see that some other DejaGNU board files increase the timeout for GCC but no others are decreasing it. I suspect this was just an oversight with the initial port of board file. I'll go ahead and remove the timeout from the board file and rerun the tests, with those dg-timeout directives also removed. Thanks, Jozef
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
On Nov 14 2018, Jozef Lawrynowicz wrote: > The timeout as set in the dejagnu configuration for msp430 > ([dejagnu.git]/baseboards/msp430-sim.exp) is 30, which is rarely > hit. I don't think it makes sense for a board file to set a smaller timeout than the default. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
On Wed, 14 Nov 2018 11:30:39 -0500
Paul Koning wrote:
> > On Nov 14, 2018, at 10:44 AM, Jozef Lawrynowicz
> > wrote:
> >
> > Patch 1 tweaks dg directives in tests specifically for msp430. Many of
> > these are extensions to existing target selectors in dg directives.
> >
> > <0001-TESTSUITE-MSP430-Tweak-dg-directives-for-msp430-elf.patch>
>
> For pr41779.c, you have
>
> +/* { dg-skip-if "int is smaller than float" { msp430-*-* } } */
>
> I take it that means: sizeof(int) < sizeof(float). That property also holds
> for pdp11 and perhaps other targets. Would it make sense to introduce a new
> effective-target flag for that check instead?
>
> paul
>
Paul,
Yes you are correct the comment implies sizeof(int) < sizeof(float).
I believe this was the only test where this property affected the test
results, so a new effective-target flag is probably only worth adding if it
affects at least a couple of tests.
On the other hand, I suppose there is no harm in adding another
check-effective-target and it at least means we'll catch failures across more
targets.
I'd be curious if the line I added the xfail to in c-c++-common/pr57371-2.c
also fails for pdp11.
The conversion to float might be getting optimized out whenever
sizeof(int) < sizeof(float).
Thanks,
Jozef
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
On 14/11/2018 18:50, Paul Koning wrote:
On Nov 14, 2018, at 1:00 PM, Jozef Lawrynowicz wrote:
On 14/11/2018 16:54, Andreas Schwab wrote:
On Nov 14 2018, Jozef Lawrynowicz wrote:
diff --git a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
index 6b1c427..71d24ce 100644
--- a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
+++ b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
@@ -1,6 +1,7 @@
/* Test __builtin_{add,sub}_overflow on {,un}signed long int. */
/* { dg-do run } */
/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
+/* { dg-timeout 120 { target msp430-*-* } } */
Are you sure you want to _decrease_ the timeout? The default is 300
seconds.
Andreas.
The timeout as set in the dejagnu configuration for msp430
([dejagnu.git]/baseboards/msp430-sim.exp) is 30, which is rarely hit. There are
some tests which pass most of of the time but will occasionally timeout so
maybe the default timeout in dejagnu is worth increasing a little as well.
Would it make sense to use { dg-timeout-factor 4 ... } instead? That would
make it explicit that you're raising rather than lowering the timeout.
paul
Thanks, I wasn't aware of that directive. Using dg-timeout-factor does seem
more appropriate in this case.
Jozef.
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
> On Nov 14, 2018, at 1:00 PM, Jozef Lawrynowicz
> wrote:
>
> On 14/11/2018 16:54, Andreas Schwab wrote:
>> On Nov 14 2018, Jozef Lawrynowicz wrote:
>>
>>> diff --git a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> index 6b1c427..71d24ce 100644
>>> --- a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> +++ b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> @@ -1,6 +1,7 @@
>>> /* Test __builtin_{add,sub}_overflow on {,un}signed long int. */
>>> /* { dg-do run } */
>>> /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
>>> +/* { dg-timeout 120 { target msp430-*-* } } */
>> Are you sure you want to _decrease_ the timeout? The default is 300
>> seconds.
>>
>> Andreas.
>>
> The timeout as set in the dejagnu configuration for msp430
> ([dejagnu.git]/baseboards/msp430-sim.exp) is 30, which is rarely hit. There
> are some tests which pass most of of the time but will occasionally timeout
> so maybe the default timeout in dejagnu is worth increasing a little as well.
Would it make sense to use { dg-timeout-factor 4 ... } instead? That would
make it explicit that you're raising rather than lowering the timeout.
paul
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
On 14/11/2018 16:54, Andreas Schwab wrote:
On Nov 14 2018, Jozef Lawrynowicz wrote:
diff --git a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
index 6b1c427..71d24ce 100644
--- a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
+++ b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
@@ -1,6 +1,7 @@
/* Test __builtin_{add,sub}_overflow on {,un}signed long int. */
/* { dg-do run } */
/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
+/* { dg-timeout 120 { target msp430-*-* } } */
Are you sure you want to _decrease_ the timeout? The default is 300
seconds.
Andreas.
The timeout as set in the dejagnu configuration for msp430
([dejagnu.git]/baseboards/msp430-sim.exp) is 30, which is rarely hit.
There are some tests which pass most of of the time but will
occasionally timeout so maybe the default timeout in dejagnu is worth
increasing a little as well.
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
On Nov 14 2018, Jozef Lawrynowicz wrote:
> diff --git a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
> b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
> index 6b1c427..71d24ce 100644
> --- a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
> +++ b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
> @@ -1,6 +1,7 @@
> /* Test __builtin_{add,sub}_overflow on {,un}signed long int. */
> /* { dg-do run } */
> /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
> +/* { dg-timeout 120 { target msp430-*-* } } */
Are you sure you want to _decrease_ the timeout? The default is 300
seconds.
Andreas.
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
> On Nov 14, 2018, at 10:44 AM, Jozef Lawrynowicz
> wrote:
>
> Patch 1 tweaks dg directives in tests specifically for msp430. Many of
> these are extensions to existing target selectors in dg directives.
>
> <0001-TESTSUITE-MSP430-Tweak-dg-directives-for-msp430-elf.patch>
For pr41779.c, you have
+/* { dg-skip-if "int is smaller than float" { msp430-*-* } } */
I take it that means: sizeof(int) < sizeof(float). That property also holds
for pdp11 and perhaps other targets. Would it make sense to introduce a new
effective-target flag for that check instead?
paul
