Patchwork [committed] Fix recgprop bug with ms_abi -> sysv memcpy call (PR rtl-optimization/57003)

login
register
mail settings
Submitter Jakub Jelinek
Date April 25, 2013, 9:56 p.m.
Message ID <20130425215630.GK28963@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/239600/
State New
Headers show

Comments

Jakub Jelinek - April 25, 2013, 9:56 p.m.
Hi!

This patch fixes a bug where a ms_abi -> sysv call to memcpy
is miscompiled.  In this case, registers %rdi and %rsi aren't call
clobbered in the ms_abi, and are in sysv ABI, thus they aren't in
the regs_invalidated_by_call regset, but are instead represented
by clobber in the pattern of the CALL_INSN.  copyprop_hardreg_forward_1
kills clobbered values early, but when seeing a SET in
CALL_INSN_FUNCTION_USAGE, it adds a new equivalence for it (%rdi vs. %rax
in this case).  When the first argument register is call clobbered, normally
the regs_invalidated_by_call handling would again kill the equivalence, and
if it isn't clobbered, then the equivalence should stay, but in this case
we need to clobber it because of the CLOBBERs.  Fixed thusly, acked by
Bernd in the PR, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk and 4.8 branch.

2013-04-25  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/57003
	* regcprop.c (copyprop_hardreg_forward_1): If ksvd.ignore_set_reg,
	call note_stores with kill_clobbered_value callback again after
	killing regs_invalidated_by_call.

	* gcc.target/i386/pr57003.c: New test.


	Jakub

Patch

--- gcc/regcprop.c.jj	2013-01-11 09:03:17.000000000 +0100
+++ gcc/regcprop.c	2013-04-25 11:43:38.472315617 +0200
@@ -1015,6 +1015,13 @@  copyprop_hardreg_forward_1 (basic_block
 	  EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi)
 	    if (regno < set_regno || regno >= set_regno + set_nregs)
 	      kill_value_regno (regno, 1, vd);
+
+	  /* If SET was seen in CALL_INSN_FUNCTION_USAGE, and SET_SRC
+	     of the SET isn't in regs_invalidated_by_call hard reg set,
+	     but instead among CLOBBERs on the CALL_INSN, we could wrongly
+	     assume the value in it is still live.  */
+	  if (ksvd.ignore_set_reg)
+	    note_stores (PATTERN (insn), kill_clobbered_value, vd);
 	}
 
       /* Notice stores.  */
--- gcc/testsuite/gcc.target/i386/pr57003.c.jj	2013-04-25 11:44:37.312994815 +0200
+++ gcc/testsuite/gcc.target/i386/pr57003.c	2013-04-25 11:44:32.851008197 +0200
@@ -0,0 +1,54 @@ 
+/* PR rtl-optimization/57003 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#define N 2001
+unsigned short *b, *c, *d;
+
+__attribute__ ((noinline, noclone)) unsigned
+bar (void)
+{
+  asm volatile ("" : : : "memory");
+  return N;
+}
+
+__attribute__ ((noinline, noclone)) unsigned short *
+baz (unsigned long x)
+{
+  if (x != N * sizeof (unsigned short) + 20)
+    __builtin_abort ();
+  asm volatile ("" : : : "memory");
+  return d;
+}
+
+__attribute__ ((ms_abi, noinline, noclone))
+foo (void)
+{
+  unsigned d;
+  unsigned short *e;
+  if ((d = bar ()))
+    {
+      e = baz (d * sizeof (unsigned short) + 20);
+      __builtin_memcpy (e, b, d * sizeof (unsigned short));
+      c = e;
+    }
+}
+
+int
+main ()
+{
+  unsigned short a[2 * N];
+  int i;
+  for (i = 0; i < 2 * N; i++)
+    a[i] = i + 1;
+  b = a;
+  d = a + N;
+  asm volatile ("" : : : "memory");
+  foo ();
+  for (i = 0; i < N; i++)
+    if (a[i] != i + 1 || a[i + N] != i + 1)
+      __builtin_abort ();
+  if (c != a + N)
+    __builtin_abort ();
+  return 0;
+}