[patch] auto-inc-dec.c: Fix auto-inc-dec. (PR rtl-optimization/44404)

[kazu at codesourcery dot com (Kazu Hirata)]

Hi,

Attached is a patch to fix a bug in auto-inc-dec.c.

Consider:

char buf[128];

extern void bar (int a, const char *p);
extern char *strcpy (char *, const char *);

void
foo (int a)
{
  if (a)
    bar (0, buf);
  strcpy (buf, "0123456789abcdefghijklmnopqrstuvwxyz");
  bar (0, buf);
}

Compile this like so:

$ arm-none-linux-gnueabi-gcc -Wall -O2 -fno-unroll-loops -c kazu.c

which results in:

/tmp/ccAsbHYh.s: Assembler messages:
/tmp/ccAsbHYh.s:40: Warning: source register same as write-back base

The assembly instruction in question is:

  strb r1, [r1], #-36

This comes from auto-inc-dec.c.  Just before the auto-inc-dec pass, we
have:

(set (mem/s/c:QI (reg/f:SI 137) [0 buf+36 S1 A32])
     (reg:QI 1 r1))

(set (reg:SI 1 r1)
     (plus:SI (reg/f:SI 137)
	      (const_int -36 [0xffffffffffffffdc])))

The auto-inc-dec pass merges these two instructions and comes up with:

(set (reg:SI 1 r1)
     (reg/f:SI 137))

(set (mem/s/c:QI (post_modify:SI (reg:SI 1 r1)
				 (plus:SI (reg:SI 1 r1)
					  (const_int -36 [0xffffffffffffffdc]))) [0 buf+36 S1 A32])
     (reg:QI 1 r1))

Notice that the first instruction in the new sequence destroys r1,
which is the source register in the first instruction in the old
sequence.

It turns out that auto-inc-dec.c:find_inc has code intended to prevent
this kind of invalid merges like so:

      /* For the post_add to work, the result_reg of the inc must not be
	 used in the mem insn since this will become the new index
	 register.  */
      if (count_occurrences (PATTERN (mem_insn.insn), inc_insn.reg_res, 1) != 0)

The problem is that this code doesn't detect a case where
inc_insn.reg_res in a different mode is used in
PATTERN (mem_insn.insn).  In our case, inc_insn.reg_res is (reg:S1 1),
and (reg:Q1 1) appears in mem_insn.insn.

This patch fixes the problem by using reg_overlap_mentioned_p instead
of count_occurrences.  I've chosen reg_overlap_mentioned_p over
reg_mentioned_p because there may be a case where:

- mem_insn.insn uses (reg:SI 1)

- inc_insn.reg_res is (reg:DI 0), spanning (reg:SI 0) and (reg:SI 1).

on a 32-bit architecture.  reg_mentioned_p wouldn't work because it
just looks at register numbers with no consideration to (reg:xx)
spanning multiple hardware registers.

Tested on arm-none-linux-gnueabi.  OK to apply?

p.s.
The tesecase triggers the bug on 4.4 but not on mainline.  However,
this bug used to occur in April, 2009 on mainline, and auto-inc-dec.c
hasn't received functional (that is, non-mechanical) changes since
then.  (This bug become latent after gcse was split into multiple
passes in late April, 2009.)

Kazu Hirata

gcc/
2010-06-07  Kazu Hirata  <kazu@codesourcery.com>

	PR rtl-optimization/44404
	* auto-inc-dec.c (find_inc): Use reg_overlap_mentioned_p instead
	of count_occurrences to see if it's safe to modify mem_insn.insn.
	gcc/testsuite/

gcc/
2010-06-07  Kazu Hirata  <kazu@codesourcery.com>

	PR rtl-optimization/44404
	* gcc.dg/pr44404.c: New.

Index: gcc/auto-inc-dec.c
===================================================================
--- gcc/auto-inc-dec.c	(revision 160319)
+++ gcc/auto-inc-dec.c	(working copy)
@@ -1068,7 +1068,7 @@ find_inc (bool first_try)
       /* For the post_add to work, the result_reg of the inc must not be
 	 used in the mem insn since this will become the new index
 	 register.  */
-      if (count_occurrences (PATTERN (mem_insn.insn), inc_insn.reg_res, 1) != 0)
+      if (reg_overlap_mentioned_p (inc_insn.reg_res, PATTERN (mem_insn.insn)))
 	{
 	  if (dump_file)
 	    fprintf (dump_file, "base reg replacement failure.\n");
Index: gcc/testsuite/gcc.dg/pr44404.c
===================================================================
--- gcc/testsuite/gcc.dg/pr44404.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr44404.c	(revision 0)
@@ -0,0 +1,35 @@
+/* PR rtl-optimization/44404
+   foo() used to be miscompiled on ARM due to a bug in auto-inc-dec.c,
+   which resulted in "strb r1, [r1], #-36".  */
+
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-unroll-loops" } */
+
+extern char *strcpy (char *, const char *);
+extern int strcmp (const char*, const char*);
+extern void abort (void);
+
+char buf[128];
+
+void __attribute__((noinline))
+bar (int a, const char *p)
+{
+  if (strcmp (p, "0123456789abcdefghijklmnopqrstuvwxyz") != 0)
+    abort ();
+}
+
+void __attribute__((noinline))
+foo (int a)
+{
+  if (a)
+    bar (0, buf);
+  strcpy (buf, "0123456789abcdefghijklmnopqrstuvwxyz");
+  bar (0, buf);
+}
+
+int
+main (void)
+{
+  foo (0);
+  return 0;
+}