Ping: PR target/59605: Create jump_around_label only if it doesn't exist

[H.J. Lu]

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.


-- 
H.J.