diff mbox

Teach var-tracking about some targets setmem/movmem patterns (PR debug/47991)

Message ID 20110304205630.GM30899@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 4, 2011, 8:56 p.m. UTC 
Hi!

s390 uses (set (mem:BLK ...) (reg:DI ...)) pattern for setmem,
but even i?86 uses (set (mem:BLK ...) (const_int ...)).
Telling var-tracking in that case that (mem:BLK) has the (reg:DI)
resp. (const_int) value is wrong and leads to mode mismatches.

So, the following patch fixes it by returning NULL from find_use_val
in that case (cselib_lookup on BLKmode MEM always returns NULL,
but in this routine we are returning cselib_lookup result of the SET_SRC
which in this case has a different mode).

Bootstrapped/regtested on x86_64-linux and i686-linux and tested on
the provided testcase in s390x-linux cross.  Ok for trunk?

2011-03-04  Jakub Jelinek  <jakub@redhat.com>

	PR debug/47991
	* var-tracking.c (find_use_val): Return NULL for
	cui->sets && cui->store_p BLKmode MEMs.

	* gcc.dg/pr47991.c: New test.


	Jakub

Comments

Alexandre Oliva March 7, 2011, 5:59 p.m. UTC | #1 
On Mar  4, 2011, Jakub Jelinek <jakub@redhat.com> wrote:

> 	PR debug/47991
> 	* var-tracking.c (find_use_val): Return NULL for
> 	cui-> sets && cui->store_p BLKmode MEMs.

> 	* gcc.dg/pr47991.c: New test.

This looks good to me, but I can't approve it.
Richard Henderson March 7, 2011, 10:04 p.m. UTC | #2 
On 03/05/2011 07:56 AM, Jakub Jelinek wrote:
> 	PR debug/47991
> 	* var-tracking.c (find_use_val): Return NULL for
> 	cui->sets && cui->store_p BLKmode MEMs.
> 
> 	* gcc.dg/pr47991.c: New test.

Ok.

> +	  /* Some targets represent memset and memcpy patterns
> +	     by (set (mem:BLK ...) (reg:[QHSD]I ...)) or
> +	     (set (mem:BLK ...) (const_int ...)) or
> +	     (set (mem:BLK ...) (mem:BLK ...)).  Don't return anything
> +	     in that case, otherwise we end up with mode mismatches.  */
> +	  if (mode == BLKmode && MEM_P (x))
> +	    return NULL;

Could you please file bug reports against the first two variants
you mention here?  These sorts of mode conflicts are bad.


r~
diff mbox

Patch

--- gcc/var-tracking.c.jj	2011-02-15 15:42:27.000000000 +0100
+++ gcc/var-tracking.c	2011-03-04 19:08:15.000000000 +0100
@@ -4784,12 +4784,19 @@  find_use_val (rtx x, enum machine_mode m
   if (cui->sets)
     {
       /* This is called after uses are set up and before stores are
-	 processed bycselib, so it's safe to look up srcs, but not
+	 processed by cselib, so it's safe to look up srcs, but not
 	 dsts.  So we look up expressions that appear in srcs or in
 	 dest expressions, but we search the sets array for dests of
 	 stores.  */
       if (cui->store_p)
 	{
+	  /* Some targets represent memset and memcpy patterns
+	     by (set (mem:BLK ...) (reg:[QHSD]I ...)) or
+	     (set (mem:BLK ...) (const_int ...)) or
+	     (set (mem:BLK ...) (mem:BLK ...)).  Don't return anything
+	     in that case, otherwise we end up with mode mismatches.  */
+	  if (mode == BLKmode && MEM_P (x))
+	    return NULL;
 	  for (i = 0; i < cui->n_sets; i++)
 	    if (cui->sets[i].dest == x)
 	      return cui->sets[i].src_elt;
--- gcc/testsuite/gcc.dg/pr47991.c.jj	2011-03-04 19:23:06.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr47991.c	2011-03-04 19:25:26.000000000 +0100
@@ -0,0 +1,25 @@ 
+/* PR debug/47991 */
+/* { dg-do compile } */
+/* { dg-options "-g -Os" } */
+
+typedef __SIZE_TYPE__ size_t;
+extern inline __attribute__ ((__always_inline__))
+void *
+memset (void *x, int y, size_t z)
+{
+  return __builtin___memset_chk (x, y, z, __builtin_object_size (x, 0));
+}
+
+void
+foo (unsigned char *x, unsigned char *y, unsigned char *z,
+     unsigned char *w, unsigned int v, int u, int t)
+{
+  int i;
+  for (i = 0; i < t; i++)
+    {
+      memset (z, x[0], v);
+      memset (w, y[0], v);
+      x += u;
+    }
+  __builtin_memcpy (z, x, u);
+}