This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][RTL-ifcvt] PR rtl-optimization/68506: Fix emitting order of insns in IF-THEN-JOIN case
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: Bernd Schmidt <bschmidt at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 27 Nov 2015 09:44:58 +0000
- Subject: Re: [PATCH][RTL-ifcvt] PR rtl-optimization/68506: Fix emitting order of insns in IF-THEN-JOIN case
- Authentication-results: sourceware.org; auth=none
- References: <5656E924 dot 4030603 at arm dot com> <56570BDB dot 9070804 at redhat dot com> <56570EAA dot 1070808 at arm dot com> <565715D9 dot 4070702 at redhat dot com> <56571895 dot 90809 at arm dot com> <5657372D dot 4080907 at arm dot com> <5657382D dot 7020804 at redhat dot com> <5657394B dot 9030201 at arm dot com>
On 26/11/15 16:54, Kyrill Tkachov wrote:
On 26/11/15 16:49, Bernd Schmidt wrote:
On 11/26/2015 05:45 PM, Kyrill Tkachov wrote:
that doesn't help, punt. */
- modified_in_a = emit_a != NULL_RTX && modified_in_p (orig_b, emit_a);
if (tmp_b && then_bb)
{
These bits I thought would be part of a followup patch (which would also guard against single_set problems), and as I mentioned I'd rather have a checking assert.
Yes, you're right. I have the checking_assert statement in the followup that I've been testing.
I'll move the deletion of these two statements there as well to minimise the changes to this patch.
I'll move these bits to that patch, re-build cc1 and commit.
Here it is.
I'm committing this to trunk.
Thanks,
Kyrill
2015-11-26 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR rtl-optimization/68506
* ifcvt.c (noce_try_cmove_arith): Try emitting the else basic block
first if emit_a exists or then_bb modifies 'b'. Reindent if-else
blocks.
2015-11-26 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR rtl-optimization/68506
* gcc.c-torture/execute/pr68506.c: New test.
Thanks for your guidance,
Kyrill
So take these deletions out and leave them for the followup, and the patch is ok everywhere. No need for a full retest given that practically the same patch has been tested already, just make sure you can build cc1.
Bernd
commit ba7633ec30e8e25d7dc1975893bf56eadf223404
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Tue Nov 24 11:49:30 2015 +0000
PR rtl-optimization/68506: Fix emitting order of insns in IF-THEN-JOIN case
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index af7a3b9..3ce9fe6 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -2220,40 +2220,38 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
}
}
- if (emit_a && modified_in_a)
- {
- modified_in_b = emit_b != NULL_RTX && modified_in_p (orig_a, emit_b);
- if (tmp_b && else_bb)
- {
- FOR_BB_INSNS (else_bb, tmp_insn)
- /* Don't check inside insn_b. We will have changed it to emit_b
- with a destination that doesn't conflict. */
- if (!(insn_b && tmp_insn == insn_b)
- && modified_in_p (orig_a, tmp_insn))
- {
- modified_in_b = true;
- break;
- }
-
- }
- if (modified_in_b)
- goto end_seq_and_fail;
-
- if (!noce_emit_bb (emit_b, else_bb, b_simple))
- goto end_seq_and_fail;
+ if (emit_a || modified_in_a)
+ {
+ modified_in_b = emit_b != NULL_RTX && modified_in_p (orig_a, emit_b);
+ if (tmp_b && else_bb)
+ {
+ FOR_BB_INSNS (else_bb, tmp_insn)
+ /* Don't check inside insn_b. We will have changed it to emit_b
+ with a destination that doesn't conflict. */
+ if (!(insn_b && tmp_insn == insn_b)
+ && modified_in_p (orig_a, tmp_insn))
+ {
+ modified_in_b = true;
+ break;
+ }
+ }
+ if (modified_in_b)
+ goto end_seq_and_fail;
- if (!noce_emit_bb (emit_a, then_bb, a_simple))
- goto end_seq_and_fail;
- }
- else
- {
- if (!noce_emit_bb (emit_a, then_bb, a_simple))
- goto end_seq_and_fail;
+ if (!noce_emit_bb (emit_b, else_bb, b_simple))
+ goto end_seq_and_fail;
- if (!noce_emit_bb (emit_b, else_bb, b_simple))
- goto end_seq_and_fail;
+ if (!noce_emit_bb (emit_a, then_bb, a_simple))
+ goto end_seq_and_fail;
+ }
+ else
+ {
+ if (!noce_emit_bb (emit_a, then_bb, a_simple))
+ goto end_seq_and_fail;
- }
+ if (!noce_emit_bb (emit_b, else_bb, b_simple))
+ goto end_seq_and_fail;
+ }
target = noce_emit_cmove (if_info, x, code, XEXP (if_info->cond, 0),
XEXP (if_info->cond, 1), a, b);
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68506.c b/gcc/testsuite/gcc.c-torture/execute/pr68506.c
new file mode 100644
index 0000000..15984ed
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr68506.c
@@ -0,0 +1,63 @@
+/* { dg-options "-fno-builtin-abort" } */
+
+int a, b, m, n, o, p, s, u, i;
+char c, q, y;
+short d;
+unsigned char e;
+static int f, h;
+static short g, r, v;
+unsigned t;
+
+extern void abort ();
+
+int
+fn1 (int p1)
+{
+ return a ? p1 : p1 + a;
+}
+
+unsigned char
+fn2 (unsigned char p1, int p2)
+{
+ return p2 >= 2 ? p1 : p1 >> p2;
+}
+
+static short
+fn3 ()
+{
+ int w, x = 0;
+ for (; p < 31; p++)
+ {
+ s = fn1 (c | ((1 && c) == c));
+ t = fn2 (s, x);
+ c = (unsigned) c > -(unsigned) ((o = (m = d = t) == p) <= 4UL) && n;
+ v = -c;
+ y = 1;
+ for (; y; y++)
+ e = v == 1;
+ d = 0;
+ for (; h != 2;)
+ {
+ for (;;)
+ {
+ if (!m)
+ abort ();
+ r = 7 - f;
+ x = e = i | r;
+ q = u * g;
+ w = b == q;
+ if (w)
+ break;
+ }
+ break;
+ }
+ }
+ return x;
+}
+
+int
+main ()
+{
+ fn3 ();
+ return 0;
+}