This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Change replace_rtx if from is a REG (PR target/70245)


Hi!

The following testcase is miscompiled on ia32, because
a peephole2 calls replace_rtx trying to replace SImode %ecx with
SImode %edx, but replace_rtx (unlike e.g. simplify_replace_rtx
or validate_replace_rtx), in addition to modifying the rtxes
in place (fine in this case) only does pointer equality check
- if (x == from) return to;, which doesn't work well especially
on hard registers, where they could be different e.g. because
of different ORIGINAL_REGNO, REG_EXPR etc., but still have
the same mode and same REGNO.  Using simplify_replace_rtx
in that peephole2 is possible, but there is a risk that function
simplifies something, turning it from a previously valid address
to invalid one, so it would have to be accomodated with
a validation check and FAIL from the peephole.  Similarly,
validate_replace_rtx needs an instruction, but the peephole2 doesn't have
one yet (and again would need FAIL handling).
If it was just one peephole2 in i386 backend, I'd certainly change just
that, but looking around, at least 2 other targets use replace_rtx
in peephole2s, and quite a few backends use replace_rtx in various places,
so maybe it is better to change replace_rtx to cope with that.

So, this is what we've converged to on IRC and passed bootstrap/regtest
on x86_64-linux and i686-linux.  Is this ok for trunk?

Or is replace_rtx really meant to do pointer equality comparison only and
shall we add another similar function that e.g. supports only hard reg
-> hard reg replacements without simplifications?  What about the case
when the hard reg appears in the expression, but with different mode?
Would that always be the bug of the caller, or shall we do something
different in that case?

2016-03-16  Jakub Jelinek  <jakub@redhat.com>
	    Richard Biener  <rguenth@suse.de>

	PR target/70245
	* rtlanal.c (replace_rtx): For REG, if from is a REG,
	return to even if only REGNO is equal, and assert
	mode is the same.

	* g++.dg/opt/pr70245.C: New test.
	* g++.dg/opt/pr70245.h: New file.
	* g++.dg/opt/pr70245-aux.cc: New file.

--- gcc/rtlanal.c.jj	2016-03-16 10:36:36.000000000 +0100
+++ gcc/rtlanal.c	2016-03-16 10:37:19.317571738 +0100
@@ -2961,7 +2961,16 @@ replace_rtx (rtx x, rtx from, rtx to)
   if (x == 0)
     return 0;
 
-  if (GET_CODE (x) == SUBREG)
+  if (GET_CODE (x) == REG)
+    {
+      if (GET_CODE (from) == REG
+	  && REGNO (x) == REGNO (from))
+	{
+	  gcc_assert (GET_MODE (x) == GET_MODE (from));
+	  return to;
+	}
+    }
+  else if (GET_CODE (x) == SUBREG)
     {
       rtx new_rtx = replace_rtx (SUBREG_REG (x), from, to);
 
--- gcc/testsuite/g++.dg/opt/pr70245.C.jj	2016-03-16 09:02:11.263150597 +0100
+++ gcc/testsuite/g++.dg/opt/pr70245.C	2016-03-16 10:38:02.408955556 +0100
@@ -0,0 +1,52 @@
+// PR target/70245
+// { dg-do run }
+// { dg-additional-sources "pr70245-aux.cc" }
+// { dg-options "-O2" }
+// { dg-additional-options "-fPIC" { target fpic } }
+// { dg-additional-options "-march=i386 -mtune=atom" { target ia32 } }
+
+#include "pr70245.h"
+
+struct A *a, *i;
+int b, c, e, l;
+F d;
+
+static A *
+foo (B *x, int *y, int *z)
+{
+  unsigned char *f = (unsigned char *) fn3 (y);
+  D *g = (D *) f;
+  A *h;
+  if (e || a || c || b || g->d)
+    return 0;
+  h = (A *) fn4 ();
+  __builtin_memcpy (h, a, sizeof (A));
+  h->a1 = *(D *) f;
+  if (d)
+    {
+      d (h, x, f + g->d, z);
+      if (*z)
+	fn2 ();
+    }
+  return h;
+}
+
+static A *
+bar (B *x, int *y)
+{
+  int *j = fn1 (x->b, y);
+  if (*y > 0)
+    return 0;
+  i = foo (x, j, y);
+  return i;
+}
+
+B k;
+
+void
+baz (int x)
+{
+  if (x)
+    bar (0, 0);
+  bar (&k, &l);
+}
--- gcc/testsuite/g++.dg/opt/pr70245.h.jj	2016-03-16 09:02:17.073069217 +0100
+++ gcc/testsuite/g++.dg/opt/pr70245.h	2016-03-15 20:19:37.000000000 +0100
@@ -0,0 +1,14 @@
+extern struct A *a, *i;
+extern int b, c, e, l;
+int *fn1 (char *, int *);
+void fn2 ();
+void *fn3 (int *);
+struct B { char *b; };
+typedef void (*F) (A *, B *, unsigned char *, int *);
+struct C { int c[16]; };
+struct D { int d; };
+struct A { D a1; C a2; };
+void *fn4 ();
+extern F d;
+extern B k;
+extern void baz (int);
--- gcc/testsuite/g++.dg/opt/pr70245-aux.cc.jj	2016-03-16 09:02:14.113110677 +0100
+++ gcc/testsuite/g++.dg/opt/pr70245-aux.cc	2016-03-16 09:05:04.803719815 +0100
@@ -0,0 +1,56 @@
+// PR target/70245
+// { dg-do compile }
+// { dg-options "" }
+
+#include "pr70245.h"
+
+D m;
+A n, o;
+int p, q;
+
+int *
+fn1 (char *x, int *y)
+{
+  *y = 0;
+  return &p;
+}
+
+void
+fn2 ()
+{
+  __builtin_abort ();
+}
+
+void *
+fn3 (int *x)
+{
+  *x = 0;
+  return (void *) &m;
+}
+
+void *
+fn4 ()
+{
+  a = &o;
+  o.a1.d = 9;
+  m.d = sizeof (D);
+  __builtin_memcpy (o.a2.c, "abcdefghijklmnop", 16);
+  return (void *) &n;
+}
+
+void
+fn5 (A *x, B *y, unsigned char *z, int *w)
+{
+  if (x != &n || y != &k || z != (unsigned char *) (&m + 1))
+    __builtin_abort ();
+  q++;
+}
+
+int
+main ()
+{
+  d = fn5;
+  baz (0);
+  if (q != 1)
+    __builtin_abort ();
+}

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]