| Message ID | 20131227023110.GA565@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 26, 2013 at 06:31:10PM -0800, H.J. Lu wrote: > 2013-12-26 H.J. Lu <hongjiu.lu@intel.com> > > PR target/59605 > * config/i386/i386.c (ix86_expand_set_or_movmem): Create > jump_around_label only if it doesn't exist. > > gcc/testsuite/ > > 2013-12-26 H.J. Lu <hongjiu.lu@intel.com> > > PR target/59605 > * gcc.dg/pr59605.c: New test. This is ok, thanks. Jakub
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? Thanks, Richard
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?
On Thu, Dec 26, 2013 at 6:31 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > Hi Honza, > > r203937 may create jump_around_label earlier. But later code doesn't > check if jump_around_label exists. This patch fixes it. Tested > on Linux/x86-64. OK to install? This test times out when running on a simulator. Is there a way to reduce the MAX_LENGTH size? Thanks, Andrew Pinski > > Thanks. > > H.J. > -- > gcc/ > > 2013-12-26 H.J. Lu <hongjiu.lu@intel.com> > > PR target/59605 > * config/i386/i386.c (ix86_expand_set_or_movmem): Create > jump_around_label only if it doesn't exist. > > gcc/testsuite/ > > 2013-12-26 H.J. Lu <hongjiu.lu@intel.com> > > PR target/59605 > * gcc.dg/pr59605.c: New test. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 0cf0a9d..07f9a86 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -24015,7 +24015,8 @@ ix86_expand_set_or_movmem (rtx dst, rtx src, rtx count_exp, rtx val_exp, > else > { > rtx hot_label = gen_label_rtx (); > - jump_around_label = gen_label_rtx (); > + if (jump_around_label == NULL_RTX) > + jump_around_label = gen_label_rtx (); > emit_cmp_and_jump_insns (count_exp, GEN_INT (dynamic_check - 1), > LEU, 0, GET_MODE (count_exp), 1, hot_label); > predict_jump (REG_BR_PROB_BASE * 90 / 100); > 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; > +}
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 0cf0a9d..07f9a86 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -24015,7 +24015,8 @@ ix86_expand_set_or_movmem (rtx dst, rtx src, rtx count_exp, rtx val_exp, else { rtx hot_label = gen_label_rtx (); - jump_around_label = gen_label_rtx (); + if (jump_around_label == NULL_RTX) + jump_around_label = gen_label_rtx (); emit_cmp_and_jump_insns (count_exp, GEN_INT (dynamic_check - 1), LEU, 0, GET_MODE (count_exp), 1, hot_label); predict_jump (REG_BR_PROB_BASE * 90 / 100); 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; +}