Patchwork Fix simplify-rtx losing side effects in operand of IOR

login
register
mail settings
Submitter Joseph S. Myers
Date Aug. 8, 2012, 9:48 a.m.
Message ID <Pine.LNX.4.64.1208080946590.27239@digraph.polyomino.org.uk>
Download mbox | patch
Permalink /patch/175886/
State New
Headers show

Comments

Joseph S. Myers - Aug. 8, 2012, 9:48 a.m.
simplify-rtx.c:simplify_binary_operation_1 simplifies certain cases of
IOR to constants, but fails to check when doing so that it hasn't lost
side effects in the nonconstant operand.  This can cause wrong code
generation (when called from combine) in certain cases where the
constant operand only becomes visible for optimization at RTL
optimization time (otherwise an equivalent optimization will happen
earlier), and the nonconstant operand is a memory operand with a
pre/post-modify address; the fix, adding side_effects_p calls, is
straightforward (and I didn't find such issues elsewhere in
simplify-rtx.c).

The above conditions make this problem quite hard to trigger, but it
shows up in pcretest on ARM with -O2 -funroll-loops, and the patch
includes an artificial testcase that shows the problem (again, -O2
-funroll-loops; or -O3 -fomit-frame-pointer -funroll-loops as used by
c-torture; I don't have an example that shows the issue without
-funroll-loops).

Tested with no regressions with cross to arm-none-linux-gnueabi
(--with-mode=thumb --with-arch=armv7-a), and bootstrapped with no
regressions on x86_64-unknown-linux-gnu.  OK to commit?

2012-08-08  Joseph Myers  <joseph@codesourcery.com>

	* simplify-rtx.c (simplify_binary_operation_1): Do not simplify
	IOR to a constant if one operand has side effects.

testsuite:
2012-08-08  Joseph Myers  <joseph@codesourcery.com>

	* gcc.c-torture/execute/20120808-1.c: New test.
Richard Sandiford - Aug. 8, 2012, 10:33 a.m.
"Joseph S. Myers" <joseph@codesourcery.com> writes:
> 2012-08-08  Joseph Myers  <joseph@codesourcery.com>
>
> 	* simplify-rtx.c (simplify_binary_operation_1): Do not simplify
> 	IOR to a constant if one operand has side effects.

OK, thanks.

Richard

Patch

Index: testsuite/gcc.c-torture/execute/20120808-1.c
===================================================================
--- testsuite/gcc.c-torture/execute/20120808-1.c	(revision 0)
+++ testsuite/gcc.c-torture/execute/20120808-1.c	(revision 0)
@@ -0,0 +1,37 @@ 
+extern void exit (int);
+extern void abort (void);
+
+volatile int i;
+unsigned char *volatile cp;
+unsigned char d[32] = { 0 };
+
+int
+main (void)
+{
+  unsigned char c[32] = { 0 };
+  unsigned char *p = d + i;
+  int j;
+  for (j = 0; j < 30; j++)
+    {
+      int x = 0xff;
+      int y = *++p;
+      switch (j)
+	{
+	case 1: x ^= 2; break;
+	case 2: x ^= 4; break;
+	case 25: x ^= 1; break;
+	default: break;
+	}
+      c[j] = y | x;
+      cp = p;
+    }
+  if (c[0] != 0xff
+      || c[1] != 0xfd
+      || c[2] != 0xfb
+      || c[3] != 0xff
+      || c[4] != 0xff
+      || c[25] != 0xfe
+      || cp != d + 30)
+    abort ();
+  exit (0);
+}
Index: simplify-rtx.c
===================================================================
--- simplify-rtx.c	(revision 190186)
+++ simplify-rtx.c	(working copy)
@@ -2420,7 +2420,9 @@  simplify_binary_operation_1 (enum rtx_co
     case IOR:
       if (trueop1 == CONST0_RTX (mode))
 	return op0;
-      if (INTEGRAL_MODE_P (mode) && trueop1 == CONSTM1_RTX (mode))
+      if (INTEGRAL_MODE_P (mode)
+	  && trueop1 == CONSTM1_RTX (mode)
+	  && !side_effects_p (op0))
 	return op1;
       if (rtx_equal_p (trueop0, trueop1) && ! side_effects_p (op0))
 	return op0;
@@ -2434,7 +2436,8 @@  simplify_binary_operation_1 (enum rtx_co
       /* (ior A C) is C if all bits of A that might be nonzero are on in C.  */
       if (CONST_INT_P (op1)
 	  && HWI_COMPUTABLE_MODE_P (mode)
-	  && (nonzero_bits (op0, mode) & ~UINTVAL (op1)) == 0)
+	  && (nonzero_bits (op0, mode) & ~UINTVAL (op1)) == 0
+	  && !side_effects_p (op0))
 	return op1;
 
       /* Canonicalize (X & C1) | C2.  */