Re: PATCH: PR target/59605: Create jump_around_label only if it doesn't exist
On Thu, Dec 26, 2013 at 6:31 PM, H.J. Lu hjl.to...@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...@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...@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 000..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;
+}
Re: PATCH: PR target/59605: Create jump_around_label only if it doesn't exist
H.J. Lu hjl.to...@gmail.com writes:
On Thu, Jan 30, 2014 at 10:43 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
Hi H.J.,
H.J. Lu hjl.to...@gmail.com writes:
diff --git a/gcc/testsuite/gcc.dg/pr59605.c b/gcc/testsuite/gcc.dg/pr59605.c
new file mode 100644
index 000..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 +
+++ gcc/testsuite/gcc.dg/pr59605-2.c2014-02-01 10:25:08.674430391 +
@@ -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 +
+++ gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:24:22.713003808 +
@@ -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)
Re: PATCH: PR target/59605: Create jump_around_label only if it doesn't exist
On Sat, Feb 1, 2014 at 2:28 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
H.J. Lu hjl.to...@gmail.com writes:
On Thu, Jan 30, 2014 at 10:43 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
Hi H.J.,
H.J. Lu hjl.to...@gmail.com writes:
diff --git a/gcc/testsuite/gcc.dg/pr59605.c
b/gcc/testsuite/gcc.dg/pr59605.c
new file mode 100644
index 000..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 +
+++ gcc/testsuite/gcc.dg/pr59605-2.c2014-02-01 10:25:08.674430391 +
@@ -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 +
+++ gcc/testsuite/gcc.dg/pr59605.c 2014-02-01 10:24:22.713003808 +
@@ -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.
Re: PATCH: PR target/59605: Create jump_around_label only if it doesn't exist
Hi H.J.,
H.J. Lu hjl.to...@gmail.com writes:
diff --git a/gcc/testsuite/gcc.dg/pr59605.c b/gcc/testsuite/gcc.dg/pr59605.c
new file mode 100644
index 000..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
Re: PATCH: PR target/59605: Create jump_around_label only if it doesn't exist
On Thu, Jan 30, 2014 at 10:43 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
Hi H.J.,
H.J. Lu hjl.to...@gmail.com writes:
diff --git a/gcc/testsuite/gcc.dg/pr59605.c b/gcc/testsuite/gcc.dg/pr59605.c
new file mode 100644
index 000..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?
--
H.J.
Re: PATCH: PR target/59605: Create jump_around_label only if it doesn't exist
On Thu, Dec 26, 2013 at 06:31:10PM -0800, H.J. Lu wrote: 2013-12-26 H.J. Lu hongjiu...@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...@intel.com PR target/59605 * gcc.dg/pr59605.c: New test. This is ok, thanks. Jakub
PATCH: PR target/59605: Create jump_around_label only if it doesn't exist
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?
Thanks.
H.J.
--
gcc/
2013-12-26 H.J. Lu hongjiu...@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...@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 000..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;
+}
