| Message ID | 55F13D66.9010207@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Kyrill, > Rainer, could you please check that this patch still fixes the SPARC > regressions? unfortunately, it breaks sparc-sun-solaris2.10 bootstrap: compiling stage2 libiberty/regex.c FAILs: In file included from /vol/gcc/src/hg/trunk/local/libiberty/regex.c:640:0: /vol/gcc/src/hg/trunk/local/libiberty/regex.c: In function 'byte_regex_compile': /vol/gcc/src/hg/trunk/local/libiberty/regex.c:4223:1: internal compiler error: in emit_move_insn, at expr.c:3552 } /* regex_compile */ ^ 0x6f1893 emit_move_insn(rtx_def*, rtx_def*) /vol/gcc/src/hg/trunk/local/gcc/expr.c:3551 0x13c524f noce_emit_move_insn /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:927 0x13c8d93 noce_try_cmove_arith /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:2026 0x13cd40b noce_process_if_block /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:3229 0x13ceeb7 noce_find_if_block /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:3678 0x13cf8fb find_if_header /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:3883 0x13d39df if_convert /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:5030 0x13d3f13 execute /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:5176 Rainer
Hi Rainer, On 10/09/15 12:43, Rainer Orth wrote: > Hi Kyrill, > >> Rainer, could you please check that this patch still fixes the SPARC >> regressions? > unfortunately, it breaks sparc-sun-solaris2.10 bootstrap: compiling > stage2 libiberty/regex.c FAILs: Thanks for trying it out. I'll try reproducing with a stage-1 cross-compiler. Any change you could send me the preprocessed regex.i? Thanks, Kyrill > In file included from /vol/gcc/src/hg/trunk/local/libiberty/regex.c:640:0: > /vol/gcc/src/hg/trunk/local/libiberty/regex.c: In function 'byte_regex_compile': > /vol/gcc/src/hg/trunk/local/libiberty/regex.c:4223:1: internal compiler error: in emit_move_insn, at expr.c:3552 > } /* regex_compile */ > ^ > 0x6f1893 emit_move_insn(rtx_def*, rtx_def*) > /vol/gcc/src/hg/trunk/local/gcc/expr.c:3551 > 0x13c524f noce_emit_move_insn > /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:927 > 0x13c8d93 noce_try_cmove_arith > /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:2026 > 0x13cd40b noce_process_if_block > /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:3229 > 0x13ceeb7 noce_find_if_block > /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:3678 > 0x13cf8fb find_if_header > /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:3883 > 0x13d39df if_convert > /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:5030 > 0x13d3f13 execute > /vol/gcc/src/hg/trunk/local/gcc/ifcvt.c:5176 > > Rainer >
commit ca25fa7000dd9d086b78bf9a02126f83fbab8073
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Mon Sep 7 14:58:01 2015 +0100
[RTL-ifcvt] PR rtl-optimization/67465: Do not ifcvt complex blocks if the else block is empty
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index d2f5b66..9adce60 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1997,6 +1997,7 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
rtx a = if_info->a;
rtx b = if_info->b;
rtx x = if_info->x;
+ machine_mode x_mode = GET_MODE (x);
rtx orig_a, orig_b;
rtx_insn *insn_a, *insn_b;
bool a_simple = if_info->then_simple;
@@ -2008,6 +2009,9 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
enum rtx_code code;
rtx_insn *ifcvt_seq;
+ if (!can_conditionally_move_p (x_mode))
+ return FALSE;
+
/* A conditional move from two memory sources is equivalent to a
conditional on their addresses followed by a load. Don't do this
early because it'll screw alias analysis. Note that we've
@@ -2079,13 +2083,32 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
}
}
- if (!a_simple && then_bb && !b_simple && else_bb
+ if (then_bb && else_bb && !a_simple && !b_simple
&& (!bbs_ok_for_cmove_arith (then_bb, else_bb)
|| !bbs_ok_for_cmove_arith (else_bb, then_bb)))
return FALSE;
start_sequence ();
+ /* If one of the blocks is empty then the corresponding B or A value
+ came from the test block. The non-empty complex block that we will
+ emit might clobber the register used by B or A, so move it to a pseudo
+ first. */
+
+ if (b_simple || !else_bb)
+ {
+ rtx tmp_b = gen_reg_rtx (x_mode);
+ noce_emit_move_insn (tmp_b, b);
+ b = tmp_b;
+ }
+
+ if (a_simple || !then_bb)
+ {
+ rtx tmp_a = gen_reg_rtx (x_mode);
+ noce_emit_move_insn (tmp_a, a);
+ a = tmp_a;
+ }
+
orig_a = a;
orig_b = b;
diff --git a/gcc/testsuite/gcc.dg/pr67465.c b/gcc/testsuite/gcc.dg/pr67465.c
new file mode 100644
index 0000000..321fd38
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr67465.c
@@ -0,0 +1,53 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -std=gnu99" } */
+
+int a, b, c, d, e, h;
+
+int
+fn1 (int p1)
+{
+ {
+ int g[2];
+ for (int i = 0; i < 1; i++)
+ g[i] = 0;
+ if (g[0] < c)
+ {
+ a = (unsigned) (1 ^ p1) % 2;
+ return 0;
+ }
+ }
+ return 0;
+}
+
+void
+fn2 ()
+{
+ for (h = 0; h < 1; h++)
+ {
+ for (int j = 0; j < 2; j++)
+ {
+ for (b = 1; b; b = 0)
+ a = 1;
+ for (; b < 1; b++)
+ ;
+ if (e)
+ continue;
+ a = 2;
+ }
+ fn1 (h);
+ short k = -16;
+ d = k > a;
+ }
+}
+
+int
+main ()
+{
+ fn2 ();
+
+ if (a != 2)
+ __builtin_abort ();
+
+ return 0;
+}
+
Hi all, This is the second attempt to fix the PRs. The first one at https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00449.html does the trick, but is overly restrictive. This one allows for cases when one block is complex and the other one is simple or empty. Earlier, this case would bypass the bbs_ok_for_cmove_arith and we would end up if-converting cases where the 'else' part (x := b) was pulled from the test block earlier in the call chain. If the reg 'b' in this case was also written to by the 'then' block, then we would miscompile. With this patch we move the original 'b' value into a pseudo before potentially clobbering it, so all is wired up properly. With this patch the PRs work for me and the restriction on if-conversion is not overly aggressive. Rainer, could you please check that this patch still fixes the SPARC regressions? I've bootstrapped and tested this on x86_64 and aarch64. Ok for trunk if SPARC testing is fine? Thanks, Kyrill 2015-09-10 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR rtl-optimization/67456 PR rtl-optimization/67464 PR rtl-optimization/67465 PR rtl-optimization/67481 * ifcvt.c (noce_try_cmove_arith): Bail out if cannot conditionally move in the mode of x. Handle combination of complex and simple block pairs as well as the case when one is empty. 2015-09-10 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * gcc.dg/pr67465.c: New test.