diff mbox

Ensure noce_convert_multiple_sets handles only multiple sets (PR rtl-optimization/69570)

Message ID 20160201083207.GB3017@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 1, 2016, 8:32 a.m. UTC
Hi!

While looking at this PR (which is most likely a reg-stack or RA bug
triggered by the ifcvt noce_convert_multiple_sets additions), I've noticed
that despite the "multiple_sets" in the name it actually attempts to handle
not just multiple sets, but also the single set case, which is already
handled by various calls later on noce_process_if_block.  Additionally,
it handles it worse than those calls we had for years, because it always
creates temporary pseudos to store result into and then at the end copy back
to the desired register.  If there is just a single set, this temporary is
unnecessary and unfortunately negatively affects RA (get larger code with
more spills/fills in *.reload/postreload).

So, I'd like to change noce_convert_multiple_sets to really apply to
multiple sets only.  While it makes the issue latent again (and I'll try to
analyze it), IMHO it is the right step forward.

Bootstrapped/regtested on {x86_64,i686,ppc64,ppc64le,s390,s390x,aarch64}-linux,
ok for trunk?

2016-02-01  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/69570
	* ifcvt.c (bb_ok_for_noce_convert_multiple_sets): Return true only
	if there is more than one set, not if there is a single set.

	* g++.dg/opt/pr69570.C: New test.


	Jakub

Comments

Steven Bosscher Feb. 1, 2016, 8:39 a.m. UTC | #1
On Mon, Feb 1, 2016 at 9:32 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Bootstrapped/regtested on {x86_64,i686,ppc64,ppc64le,s390,s390x,aarch64}-linux,
> ok for trunk?

OK.

Browny points for opting out of the loop over all insns in the basic
block when count > limit.

Ciao!
Steven
diff mbox

Patch

--- gcc/ifcvt.c.jj	2016-01-21 17:53:32.000000000 +0100
+++ gcc/ifcvt.c	2016-01-31 13:47:34.171323086 +0100
@@ -3295,7 +3295,7 @@  bb_ok_for_noce_convert_multiple_sets (ba
   if (count > limit)
     return false;
 
-  return count > 0;
+  return count > 1;
 }
 
 /* Given a simple IF-THEN-JOIN or IF-THEN-ELSE-JOIN block, attempt to convert
--- gcc/testsuite/g++.dg/opt/pr69570.C.jj	2016-01-31 22:49:03.747216450 +0100
+++ gcc/testsuite/g++.dg/opt/pr69570.C	2016-01-31 22:49:18.861009011 +0100
@@ -0,0 +1,70 @@ 
+// PR rtl-optimization/69570
+// { dg-do run }
+// { dg-options "-O2" }
+// { dg-additional-options "-fpic" { target fpic } }
+// { dg-additional-options "-march=i686" { target ia32 } }
+
+template <typename T> inline const T &
+min (const T &a, const T &b)
+{
+  if (b < a)
+    return b;
+  return a;
+}
+
+template <typename T> inline const T &
+max (const T &a, const T &b)
+{
+  if (a < b)
+    return b;
+  return a;
+}
+
+static inline void
+foo (unsigned x, unsigned y, unsigned z, double &h, double &s, double &l)
+{
+  double r = x / 255.0;
+  double g = y / 255.0;
+  double b = z / 255.0;
+  double m = max (r, max (g, b));
+  double n = min (r, min (g, b));
+  double d = m - n;
+  double e = m + n;
+  h = 0.0, s = 0.0, l = e / 2.0;
+  if (d > 0.0)
+    {
+      s = l > 0.5 ? d / (2.0 - e) : d / e;
+      if (m == r && m != g)
+        h = (g - b) / d + (g < b ? 6.0 : 0.0);
+      if (m == g && m != b)
+        h = (b - r) / d + 2.0;
+      if (m == b && m != r)
+        h = (r - g) / d + 4.0;
+      h /= 6.0;
+    }
+}
+
+__attribute__ ((noinline, noclone))
+void bar (unsigned x[3], double y[3])
+{
+  double h, s, l;
+  foo (x[0], x[1], x[2], h, s, l);
+  y[0] = h;
+  y[1] = s;
+  y[2] = l;
+}
+
+int
+main ()
+{
+  unsigned x[3] = { 0, 128, 0 };
+  double y[3];
+
+  bar (x, y);
+  if (__builtin_fabs (y[0] - 0.33333) > 0.001
+      || __builtin_fabs (y[1] - 1) > 0.001
+      || __builtin_fabs (y[2] - 0.25098) > 0.001)
+    __builtin_abort ();
+
+  return 0;
+}