public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][RTL-ifcvt] PR rtl-optimization/67465: Handle pairs of complex+simple blocks and empty blocks more gracefully
@ 2015-09-10  8:23 Kyrill Tkachov
  2015-09-10 11:57 ` Rainer Orth
  0 siblings, 1 reply; 14+ messages in thread
From: Kyrill Tkachov @ 2015-09-10  8:23 UTC (permalink / raw)
  To: GCC Patches; +Cc: Rainer Orth, Jeff Law

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ifcvt-bug-fix.patch --]
[-- Type: text/x-patch; name=ifcvt-bug-fix.patch, Size: 2863 bytes --]

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;
+}
+

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-09-28 16:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-10  8:23 [PATCH][RTL-ifcvt] PR rtl-optimization/67465: Handle pairs of complex+simple blocks and empty blocks more gracefully Kyrill Tkachov
2015-09-10 11:57 ` Rainer Orth
2015-09-10 13:18   ` Kyrill Tkachov
2015-09-10 14:52   ` Kyrill Tkachov
2015-09-11  8:53     ` Rainer Orth
2015-09-11 15:43       ` Kyrill Tkachov
2015-09-17 12:02         ` Rainer Orth
2015-09-17 16:36           ` Kyrill Tkachov
2015-09-18  9:24             ` Rainer Orth
2015-09-25 11:20             ` Rainer Orth
2015-09-25 11:21               ` Kyrill Tkachov
2015-09-25 20:32                 ` Jeff Law
2015-09-28  9:43                   ` Kyrill Tkachov
2015-09-28 17:09                     ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).