[PATCH] combine: Fix for PR67028

[PATCH] combine: Fix for PR67028

[Segher Boessenkool]

The transformation

	  /* If this is (and:M1 (subreg:M1 X:M2 0) (const_int C1)) where C1
	     fits in both M1 and M2 and the SUBREG is either paradoxical
	     or represents the low part, permute the SUBREG and the AND
	     and try again.  */

in the paradoxical case ("M1 > M2"), which implies WORD_REGISTER_OPERATIONS,
does not work if C1 changes value when in mode M2 (because it does not
express the same value for the top bits of the M1 result).  This fixes it.

Bootstrapped and tested on powerpc64-linux.  Applying to trunk; will
backport it in a week or so.


Segher


2015-07-27  Segher Boessenkool  <segher@kernel.crashing.org>

	PR rtl-optimization/67028
	* combine.c (simplify_comparison): Fix comment.  Rearrange code.
	Add test to see if a const_int fits in the new mode.

gcc/testsuite/
	PR rtl-optimization/67028
	* gcc.dg/pr67028.c: New testcase.

---
 gcc/combine.c                  | 19 ++++++++++++-------
 gcc/testsuite/gcc.dg/pr67028.c | 21 +++++++++++++++++++++
 2 files changed, 33 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr67028.c

diff --git a/gcc/combine.c b/gcc/combine.c
index 4a92f55..8f98cbb 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -12057,14 +12057,15 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
 	      continue;
 	    }
 
-	  /* If this is (and:M1 (subreg:M2 X 0) (const_int C1)) where C1
+	  /* If this is (and:M1 (subreg:M1 X:M2 0) (const_int C1)) where C1
 	     fits in both M1 and M2 and the SUBREG is either paradoxical
 	     or represents the low part, permute the SUBREG and the AND
 	     and try again.  */
-	  if (GET_CODE (XEXP (op0, 0)) == SUBREG)
+	  if (GET_CODE (XEXP (op0, 0)) == SUBREG
+	      && CONST_INT_P (XEXP (op0, 1)))
 	    {
-	      unsigned HOST_WIDE_INT c1;
 	      tmode = GET_MODE (SUBREG_REG (XEXP (op0, 0)));
+	      unsigned HOST_WIDE_INT c1 = INTVAL (XEXP (op0, 1));
 	      /* Require an integral mode, to avoid creating something like
 		 (AND:SF ...).  */
 	      if (SCALAR_INT_MODE_P (tmode)
@@ -12074,16 +12075,20 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
 		     have a defined value due to the AND operation.
 		     However, if we commute the AND inside the SUBREG then
 		     they no longer have defined values and the meaning of
-		     the code has been changed.  */
+		     the code has been changed.
+		     Also C1 should not change value in the smaller mode,
+		     see PR67028 (a positive C1 can become negative in the
+		     smaller mode, so that the AND does no longer mask the
+		     upper bits).  */
 		  && ((WORD_REGISTER_OPERATIONS
 		       && mode_width > GET_MODE_PRECISION (tmode)
-		       && mode_width <= BITS_PER_WORD)
+		       && mode_width <= BITS_PER_WORD
+		       && trunc_int_for_mode (c1, tmode) == (HOST_WIDE_INT) c1)
 		      || (mode_width <= GET_MODE_PRECISION (tmode)
 			  && subreg_lowpart_p (XEXP (op0, 0))))
-		  && CONST_INT_P (XEXP (op0, 1))
 		  && mode_width <= HOST_BITS_PER_WIDE_INT
 		  && HWI_COMPUTABLE_MODE_P (tmode)
-		  && ((c1 = INTVAL (XEXP (op0, 1))) & ~mask) == 0
+		  && (c1 & ~mask) == 0
 		  && (c1 & ~GET_MODE_MASK (tmode)) == 0
 		  && c1 != mask
 		  && c1 != GET_MODE_MASK (tmode))
diff --git a/gcc/testsuite/gcc.dg/pr67028.c b/gcc/testsuite/gcc.dg/pr67028.c
new file mode 100644
index 0000000..b42fb81
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr67028.c
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+/* { dg-options "-O3" } */
+
+short c = 0;
+
+int __attribute__ ((noinline)) f(void)
+{
+	int d = 5;
+	signed char e = (c != 1) * -2;
+	int a = (unsigned short)e > d;
+
+	return a;
+}
+
+int main(void)
+{
+	if (!f())
+		__builtin_abort();
+
+	return 0;
+}
-- 
1.8.1.4

Re: [PATCH] combine: Fix for PR67028

[Segher Boessenkool]

On Fri, Aug 07, 2015 at 06:43:28PM -0700, Segher Boessenkool wrote:
> Bootstrapped and tested on powerpc64-linux.  Applying to trunk; will
> backport it in a week or so.

I did that now (5 and 4.9).  The patch needed a teeny bit of wiggling
because the older branches still have #ifdef WORD_REGISTER_OPERATIONS.


Segher