Message ID | 87y51v9lf8.fsf@talisman.default |
---|---|
State | New |
Headers | show |
On Sat, Feb 1, 2014 at 2:28 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: >> On Thu, Jan 30, 2014 at 10:43 AM, Richard Sandiford >> <rdsandiford@googlemail.com> wrote: >>> Hi H.J., >>> >>> "H.J. Lu" <hjl.tools@gmail.com> writes: >>>> diff --git a/gcc/testsuite/gcc.dg/pr59605.c b/gcc/testsuite/gcc.dg/pr59605.c >>>> new file mode 100644 >>>> index 0000000..4556843 >>>> --- /dev/null >>>> +++ b/gcc/testsuite/gcc.dg/pr59605.c >>>> @@ -0,0 +1,55 @@ >>>> +/* { dg-do run } */ >>>> +/* { dg-options "-O2" } */ >>>> +/* { dg-additional-options "-minline-stringops-dynamically" { target >>>> { i?86-*-* x86_64-*-* } } } */ >>>> + >>>> +extern void abort (void); >>>> + >>>> +#define MAX_OFFSET (sizeof (long long)) >>>> +#define MAX_COPY (1024 + 8192) >>>> +#define MAX_EXTRA (sizeof (long long)) >>>> + >>>> +#define MAX_LENGTH (MAX_OFFSET + MAX_COPY + MAX_EXTRA) >>>> + >>>> +static union { >>>> + char buf[MAX_LENGTH]; >>>> + long long align_int; >>>> + long double align_fp; >>>> +} u; >>>> + >>>> +char A[MAX_LENGTH]; >>>> + >>>> +int >>>> +main () >>>> +{ >>>> + int off, len, i; >>>> + char *p, *q; >>>> + >>>> + for (i = 0; i < MAX_LENGTH; i++) >>>> + A[i] = 'A'; >>>> + >>>> + for (off = 0; off < MAX_OFFSET; off++) >>>> + for (len = 1; len < MAX_COPY; len++) >>>> + { >>>> + for (i = 0; i < MAX_LENGTH; i++) >>>> + u.buf[i] = 'a'; >>>> + >>>> + p = __builtin_memcpy (u.buf + off, A, len); >>>> + if (p != u.buf + off) >>>> + abort (); >>>> + >>>> + q = u.buf; >>>> + for (i = 0; i < off; i++, q++) >>>> + if (*q != 'a') >>>> + abort (); >>>> + >>>> + for (i = 0; i < len; i++, q++) >>>> + if (*q != 'A') >>>> + abort (); >>>> + >>>> + for (i = 0; i < MAX_EXTRA; i++, q++) >>>> + if (*q != 'a') >>>> + abort (); >>>> + } >>>> + >>>> + return 0; >>>> +} >>> >>> The innermost loop bodies are executed over 6x10⁸ times on most targets >>> and so the test times out for me when using the GDB MIPS simulator. >>> I'm not sure what the best fix is though. E.g.: >>> >>> 1. It looks like the PR was for a compile-time failure rather than a run-time >>> failure, so one option might be to drop it to dg-do compile. That'd lose >>> a nice executable conformance test though. I don't like this option. >>> >>> 2. We could drop it to dg-do compile for simulator targets only. That's still >>> lose some conformance testing for simulator targets. >>> >>> 3. We could use a smaller MAX_COPY for simulator targets, which is typically >>> how check_effective_target_simulator is used. I'm not sure whether having >>> a smaller MAX_COPY would defeat the original ICE test though. >>> >>> 4. We could split the test into two, a dg-do compile one and a dg-do run one. >>> We could then do (3) on the run one. >>> >>> But there are probably other alternatives too. >>> >>> I'm willing to do the patch, but has anyone got any suggestions for >>> what would be best? >>> >> >> Can you reduce the loop count and still trigger the bug without >> the fix? > > Not by much. AFAICT the lowest MAX_COPY for which the ICE still triggers > is 8194, which only reduces the 6x10⁸ to something over 5.38x10⁸. > > So I think it's a choice between (2) and (4). How about the patch below? > Tested on mipsisa64-sde-elf and x86_64-linux-gnu. Both tests failed before > the i386.c fix. > > Thanks, > Richard > > > gcc/testsuite/ > * gcc.dg/pr59605.c: Convert to a compile test. Protect MAX_COPY > definition with an ifdef. > * gcc.dg/pr59605-2.c: New test. > > Index: gcc/testsuite/gcc.dg/pr59605-2.c > =================================================================== > --- /dev/null 2014-01-30 08:06:21.701666182 +0000 > +++ gcc/testsuite/gcc.dg/pr59605-2.c 2014-02-01 10:25:08.674430391 +0000 > @@ -0,0 +1,6 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2" } */ > +/* { dg-additional-options "-DMAX_COPY=1025" { target simulator } } */ > +/* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ > + > +#include "pr59605.c" > Index: gcc/testsuite/gcc.dg/pr59605.c > =================================================================== > --- gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:13:26.176018090 +0000 > +++ gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:24:22.713003808 +0000 > @@ -1,11 +1,13 @@ > -/* { dg-do run } */ > +/* { dg-do compile } */ > /* { dg-options "-O2" } */ > /* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ > > extern void abort (void); > > #define MAX_OFFSET (sizeof (long long)) > +#ifndef MAX_COPY > #define MAX_COPY (1024 + 8192) > +#endif > #define MAX_EXTRA (sizeof (long long)) > > #define MAX_LENGTH (MAX_OFFSET + MAX_COPY + MAX_EXTRA) That is fine with me. Thanks.
"H.J. Lu" <hjl.tools@gmail.com> writes: > On Sat, Feb 1, 2014 at 2:28 AM, Richard Sandiford > <rdsandiford@googlemail.com> wrote: >> "H.J. Lu" <hjl.tools@gmail.com> writes: >>> On Thu, Jan 30, 2014 at 10:43 AM, Richard Sandiford >>> <rdsandiford@googlemail.com> wrote: >>>> Hi H.J., >>>> >>>> "H.J. Lu" <hjl.tools@gmail.com> writes: >>>>> diff --git a/gcc/testsuite/gcc.dg/pr59605.c >>>>> b/gcc/testsuite/gcc.dg/pr59605.c >>>>> new file mode 100644 >>>>> index 0000000..4556843 >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.dg/pr59605.c >>>>> @@ -0,0 +1,55 @@ >>>>> +/* { dg-do run } */ >>>>> +/* { dg-options "-O2" } */ >>>>> +/* { dg-additional-options "-minline-stringops-dynamically" { target >>>>> { i?86-*-* x86_64-*-* } } } */ >>>>> + >>>>> +extern void abort (void); >>>>> + >>>>> +#define MAX_OFFSET (sizeof (long long)) >>>>> +#define MAX_COPY (1024 + 8192) >>>>> +#define MAX_EXTRA (sizeof (long long)) >>>>> + >>>>> +#define MAX_LENGTH (MAX_OFFSET + MAX_COPY + MAX_EXTRA) >>>>> + >>>>> +static union { >>>>> + char buf[MAX_LENGTH]; >>>>> + long long align_int; >>>>> + long double align_fp; >>>>> +} u; >>>>> + >>>>> +char A[MAX_LENGTH]; >>>>> + >>>>> +int >>>>> +main () >>>>> +{ >>>>> + int off, len, i; >>>>> + char *p, *q; >>>>> + >>>>> + for (i = 0; i < MAX_LENGTH; i++) >>>>> + A[i] = 'A'; >>>>> + >>>>> + for (off = 0; off < MAX_OFFSET; off++) >>>>> + for (len = 1; len < MAX_COPY; len++) >>>>> + { >>>>> + for (i = 0; i < MAX_LENGTH; i++) >>>>> + u.buf[i] = 'a'; >>>>> + >>>>> + p = __builtin_memcpy (u.buf + off, A, len); >>>>> + if (p != u.buf + off) >>>>> + abort (); >>>>> + >>>>> + q = u.buf; >>>>> + for (i = 0; i < off; i++, q++) >>>>> + if (*q != 'a') >>>>> + abort (); >>>>> + >>>>> + for (i = 0; i < len; i++, q++) >>>>> + if (*q != 'A') >>>>> + abort (); >>>>> + >>>>> + for (i = 0; i < MAX_EXTRA; i++, q++) >>>>> + if (*q != 'a') >>>>> + abort (); >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>> >>>> The innermost loop bodies are executed over 6x10⁸ times on most targets >>>> and so the test times out for me when using the GDB MIPS simulator. >>>> I'm not sure what the best fix is though. E.g.: >>>> >>>> 1. It looks like the PR was for a compile-time failure rather than a >>>> run-time >>>> failure, so one option might be to drop it to dg-do compile. That'd lose >>>> a nice executable conformance test though. I don't like this option. >>>> >>>> 2. We could drop it to dg-do compile for simulator targets only. >>>> That's still >>>> lose some conformance testing for simulator targets. >>>> >>>> 3. We could use a smaller MAX_COPY for simulator targets, which is typically >>>> how check_effective_target_simulator is used. I'm not sure >>>> whether having >>>> a smaller MAX_COPY would defeat the original ICE test though. >>>> >>>> 4. We could split the test into two, a dg-do compile one and a dg-do >>>> run one. >>>> We could then do (3) on the run one. >>>> >>>> But there are probably other alternatives too. >>>> >>>> I'm willing to do the patch, but has anyone got any suggestions for >>>> what would be best? >>>> >>> >>> Can you reduce the loop count and still trigger the bug without >>> the fix? >> >> Not by much. AFAICT the lowest MAX_COPY for which the ICE still triggers >> is 8194, which only reduces the 6x10⁸ to something over 5.38x10⁸. >> >> So I think it's a choice between (2) and (4). How about the patch below? >> Tested on mipsisa64-sde-elf and x86_64-linux-gnu. Both tests failed before >> the i386.c fix. >> >> Thanks, >> Richard >> >> >> gcc/testsuite/ >> * gcc.dg/pr59605.c: Convert to a compile test. Protect MAX_COPY >> definition with an ifdef. >> * gcc.dg/pr59605-2.c: New test. >> >> Index: gcc/testsuite/gcc.dg/pr59605-2.c >> =================================================================== >> --- /dev/null 2014-01-30 08:06:21.701666182 +0000 >> +++ gcc/testsuite/gcc.dg/pr59605-2.c 2014-02-01 10:25:08.674430391 +0000 >> @@ -0,0 +1,6 @@ >> +/* { dg-do run } */ >> +/* { dg-options "-O2" } */ >> +/* { dg-additional-options "-DMAX_COPY=1025" { target simulator } } */ >> +/* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ >> + >> +#include "pr59605.c" >> Index: gcc/testsuite/gcc.dg/pr59605.c >> =================================================================== >> --- gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:13:26.176018090 +0000 >> +++ gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:24:22.713003808 +0000 >> @@ -1,11 +1,13 @@ >> -/* { dg-do run } */ >> +/* { dg-do compile } */ >> /* { dg-options "-O2" } */ >> /* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ >> >> extern void abort (void); >> >> #define MAX_OFFSET (sizeof (long long)) >> +#ifndef MAX_COPY >> #define MAX_COPY (1024 + 8192) >> +#endif >> #define MAX_EXTRA (sizeof (long long)) >> >> #define MAX_LENGTH (MAX_OFFSET + MAX_COPY + MAX_EXTRA) > > That is fine with me. Thanks. Is it OK to commit? Richard
On Mon, Feb 10, 2014 at 11:54 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: >> On Sat, Feb 1, 2014 at 2:28 AM, Richard Sandiford >> <rdsandiford@googlemail.com> wrote: >>> "H.J. Lu" <hjl.tools@gmail.com> writes: >>>> On Thu, Jan 30, 2014 at 10:43 AM, Richard Sandiford >>>> <rdsandiford@googlemail.com> wrote: >>>>> Hi H.J., >>>>> >>>>> "H.J. Lu" <hjl.tools@gmail.com> writes: >>>>>> diff --git a/gcc/testsuite/gcc.dg/pr59605.c >>>>>> b/gcc/testsuite/gcc.dg/pr59605.c >>>>>> new file mode 100644 >>>>>> index 0000000..4556843 >>>>>> --- /dev/null >>>>>> +++ b/gcc/testsuite/gcc.dg/pr59605.c >>>>>> @@ -0,0 +1,55 @@ >>>>>> +/* { dg-do run } */ >>>>>> +/* { dg-options "-O2" } */ >>>>>> +/* { dg-additional-options "-minline-stringops-dynamically" { target >>>>>> { i?86-*-* x86_64-*-* } } } */ >>>>>> + >>>>>> +extern void abort (void); >>>>>> + >>>>>> +#define MAX_OFFSET (sizeof (long long)) >>>>>> +#define MAX_COPY (1024 + 8192) >>>>>> +#define MAX_EXTRA (sizeof (long long)) >>>>>> + >>>>>> +#define MAX_LENGTH (MAX_OFFSET + MAX_COPY + MAX_EXTRA) >>>>>> + >>>>>> +static union { >>>>>> + char buf[MAX_LENGTH]; >>>>>> + long long align_int; >>>>>> + long double align_fp; >>>>>> +} u; >>>>>> + >>>>>> +char A[MAX_LENGTH]; >>>>>> + >>>>>> +int >>>>>> +main () >>>>>> +{ >>>>>> + int off, len, i; >>>>>> + char *p, *q; >>>>>> + >>>>>> + for (i = 0; i < MAX_LENGTH; i++) >>>>>> + A[i] = 'A'; >>>>>> + >>>>>> + for (off = 0; off < MAX_OFFSET; off++) >>>>>> + for (len = 1; len < MAX_COPY; len++) >>>>>> + { >>>>>> + for (i = 0; i < MAX_LENGTH; i++) >>>>>> + u.buf[i] = 'a'; >>>>>> + >>>>>> + p = __builtin_memcpy (u.buf + off, A, len); >>>>>> + if (p != u.buf + off) >>>>>> + abort (); >>>>>> + >>>>>> + q = u.buf; >>>>>> + for (i = 0; i < off; i++, q++) >>>>>> + if (*q != 'a') >>>>>> + abort (); >>>>>> + >>>>>> + for (i = 0; i < len; i++, q++) >>>>>> + if (*q != 'A') >>>>>> + abort (); >>>>>> + >>>>>> + for (i = 0; i < MAX_EXTRA; i++, q++) >>>>>> + if (*q != 'a') >>>>>> + abort (); >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>> >>>>> The innermost loop bodies are executed over 6x10⁸ times on most targets >>>>> and so the test times out for me when using the GDB MIPS simulator. >>>>> I'm not sure what the best fix is though. E.g.: >>>>> >>>>> 1. It looks like the PR was for a compile-time failure rather than a >>>>> run-time >>>>> failure, so one option might be to drop it to dg-do compile. That'd lose >>>>> a nice executable conformance test though. I don't like this option. >>>>> >>>>> 2. We could drop it to dg-do compile for simulator targets only. >>>>> That's still >>>>> lose some conformance testing for simulator targets. >>>>> >>>>> 3. We could use a smaller MAX_COPY for simulator targets, which is typically >>>>> how check_effective_target_simulator is used. I'm not sure >>>>> whether having >>>>> a smaller MAX_COPY would defeat the original ICE test though. >>>>> >>>>> 4. We could split the test into two, a dg-do compile one and a dg-do >>>>> run one. >>>>> We could then do (3) on the run one. >>>>> >>>>> But there are probably other alternatives too. >>>>> >>>>> I'm willing to do the patch, but has anyone got any suggestions for >>>>> what would be best? >>>>> >>>> >>>> Can you reduce the loop count and still trigger the bug without >>>> the fix? >>> >>> Not by much. AFAICT the lowest MAX_COPY for which the ICE still triggers >>> is 8194, which only reduces the 6x10⁸ to something over 5.38x10⁸. >>> >>> So I think it's a choice between (2) and (4). How about the patch below? >>> Tested on mipsisa64-sde-elf and x86_64-linux-gnu. Both tests failed before >>> the i386.c fix. >>> >>> Thanks, >>> Richard >>> >>> >>> gcc/testsuite/ >>> * gcc.dg/pr59605.c: Convert to a compile test. Protect MAX_COPY >>> definition with an ifdef. >>> * gcc.dg/pr59605-2.c: New test. >>> >>> Index: gcc/testsuite/gcc.dg/pr59605-2.c >>> =================================================================== >>> --- /dev/null 2014-01-30 08:06:21.701666182 +0000 >>> +++ gcc/testsuite/gcc.dg/pr59605-2.c 2014-02-01 10:25:08.674430391 +0000 >>> @@ -0,0 +1,6 @@ >>> +/* { dg-do run } */ >>> +/* { dg-options "-O2" } */ >>> +/* { dg-additional-options "-DMAX_COPY=1025" { target simulator } } */ >>> +/* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ >>> + >>> +#include "pr59605.c" >>> Index: gcc/testsuite/gcc.dg/pr59605.c >>> =================================================================== >>> --- gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:13:26.176018090 +0000 >>> +++ gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:24:22.713003808 +0000 >>> @@ -1,11 +1,13 @@ >>> -/* { dg-do run } */ >>> +/* { dg-do compile } */ >>> /* { dg-options "-O2" } */ >>> /* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ >>> >>> extern void abort (void); >>> >>> #define MAX_OFFSET (sizeof (long long)) >>> +#ifndef MAX_COPY >>> #define MAX_COPY (1024 + 8192) >>> +#endif >>> #define MAX_EXTRA (sizeof (long long)) >>> >>> #define MAX_LENGTH (MAX_OFFSET + MAX_COPY + MAX_EXTRA) >> >> That is fine with me. > > Thanks. Is it OK to commit? > This is my testcase. I think you can check it in.
Index: gcc/testsuite/gcc.dg/pr59605-2.c =================================================================== --- /dev/null 2014-01-30 08:06:21.701666182 +0000 +++ gcc/testsuite/gcc.dg/pr59605-2.c 2014-02-01 10:25:08.674430391 +0000 @@ -0,0 +1,6 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ +/* { dg-additional-options "-DMAX_COPY=1025" { target simulator } } */ +/* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ + +#include "pr59605.c" Index: gcc/testsuite/gcc.dg/pr59605.c =================================================================== --- gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:13:26.176018090 +0000 +++ gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:24:22.713003808 +0000 @@ -1,11 +1,13 @@ -/* { dg-do run } */ +/* { dg-do compile } */ /* { dg-options "-O2" } */ /* { dg-additional-options "-minline-stringops-dynamically" { target { i?86-*-* x86_64-*-* } } } */ extern void abort (void); #define MAX_OFFSET (sizeof (long long)) +#ifndef MAX_COPY #define MAX_COPY (1024 + 8192) +#endif #define MAX_EXTRA (sizeof (long long)) #define MAX_LENGTH (MAX_OFFSET + MAX_COPY + MAX_EXTRA)