[PATCH, rtl-optimization]: Fix PR 51821, 64bit > 32bit conversion produces incorrect results with optimizations

Uros Bizjak ubizjak@gmail.com
Thu Jan 12 19:14:00 GMT 2012


Hello!

As discussed in the PR, attached testcase uncovers problem with the
way peephole2 pas determines free registers to allocate temporary
register. The problem was with the pattern such as:

(insn 7 19 16 2 (parallel [
            (set (reg:DI 0 ax [65])
                (ashift:DI (const_int -1 [0xffffffffffffffff])
                    (reg:QI 2 cx [orig:63 shift_size ] [63])))
            (clobber (reg:CC 17 flags))
        ]) pr51821.c:8 489 {*ashldi3_doubleword}
     (expr_list:REG_DEAD (reg:QI 2 cx [orig:63 shift_size ] [63])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (expr_list:REG_UNUSED (reg:SI 1 dx)
                (expr_list:REG_EQUAL (ashift:DI (const_int -1
[0xffffffffffffffff])
                        (subreg:QI (reg/v:SI 2 cx [orig:63 shift_size
] [63]) 0))
                    (nil))))))

where DImode ax/dx register pair gets defined, while at the same time,
one of the registers from the register pair gets marked as unused.
Currently, peephole2 pass determines clobbered registers from DF life
analysis, as the difference of registers, live before the insn
sequence and after insn sequence. Unfortunately, as shown by attached
testcase in the PR, this approach is not correct. In the above
pattern, dx is considered "dead" due to (REG_UNUSED dx) note.

The solution is to fix the scanning loop to look into the insn pattern
itself for all set and clobbered hard registers. This way, all
registers, clobbered by the pattern, will be correctly marked in the
"live" bitmap, including FLAGS register that is ignored by current
approach.

2012-01-12  Uros Bizjak  <ubizjak@gmail.com>

	* recog.c (peep2_find_free_register): Determine clobbered registers
	from insn pattern.

testsuite/ChangeLog:

2012-01-12  Uros Bizjak  <ubizjak@gmail.com>

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

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu.

OK for mainline and release branches?

Uros.
-------------- next part --------------
Index: recog.c
===================================================================
--- recog.c	(revision 183130)
+++ recog.c	(working copy)
@@ -3038,6 +3038,7 @@ peep2_find_free_register (int from, int to, const
   static int search_ofs;
   enum reg_class cl;
   HARD_REG_SET live;
+  df_ref *def_rec;
   int i;
 
   gcc_assert (from < MAX_INSNS_PER_PEEP2 + 1);
@@ -3051,12 +3052,14 @@ peep2_find_free_register (int from, int to, const
 
   while (from != to)
     {
-      HARD_REG_SET this_live;
+      gcc_assert (peep2_insn_data[from].insn != NULL_RTX);
 
+      /* Don't use registers set or clobbered by the insn.  */
+      for (def_rec = DF_INSN_DEFS (peep2_insn_data[from].insn);
+	   *def_rec; def_rec++)
+	SET_HARD_REG_BIT (live, DF_REF_REGNO (*def_rec));
+
       from = peep2_buf_position (from + 1);
-      gcc_assert (peep2_insn_data[from].insn != NULL_RTX);
-      REG_SET_TO_HARD_REG_SET (this_live, peep2_insn_data[from].live_before);
-      IOR_HARD_REG_SET (live, this_live);
     }
 
   cl = (class_str[0] == 'r' ? GENERAL_REGS
Index: testsuite/gcc.dg/pr51821.c
===================================================================
--- testsuite/gcc.dg/pr51821.c	(revision 0)
+++ testsuite/gcc.dg/pr51821.c	(revision 0)
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -msse" { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-require-effective-target sse_runtime { target { i?86-*-* x86_64-*-* } } } */
+
+extern void abort (void);
+
+unsigned int  __attribute__((noinline))
+test (int shift_size)
+{
+  unsigned long long res = ~0;
+
+  return res << shift_size;
+}
+
+int
+main ()
+{
+  int dst = 32;
+
+  if (test (dst) != 0)
+    abort ();
+
+  return 0;
+}


More information about the Gcc-patches mailing list