diff mbox series

[3/4,rtl] : simplify boolean vector EQ and NE comparisons

Message ID ZtdGkjP/lMTJwbfX@arm.com
State New
Headers show
Series [1/4] middle-end: have vect_recog_cond_store_pattern use pattern statement for cond if available | expand

Commit Message

Tamar Christina Sept. 3, 2024, 5:25 p.m. UTC 
Hi All,

This adds vector constant simplification for EQ and NE.  This is useful since
the vectorizer generates a lot more vector compares now, in particular NE and EQ
and so these help us optimize cases where the values were not known at GIMPLE
but instead only at RTL.

Bootstrapped Regtested on aarch64-none-linux-gnu, arm-none-linux-gnueabihf,
x86_64-pc-linux-gnu -m32, -m64 and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	* simplify-rtx.cc (simplify_context::simplify_unary_operation): Try
	simplifying operand.
	(simplify_const_relational_operation): Simplify vector EQ and NE.
	(test_vector_int_const_compare): New.
	(test_vector_int_const_compare_ops): New.
	(simplify_rtx_cc_tests): Use them.

---




--

Comments

Richard Sandiford Sept. 6, 2024, 1:20 p.m. UTC | #1 
Tamar Christina <tamar.christina@arm.com> writes:
> Hi All,
>
> This adds vector constant simplification for EQ and NE.  This is useful since
> the vectorizer generates a lot more vector compares now, in particular NE and EQ
> and so these help us optimize cases where the values were not known at GIMPLE
> but instead only at RTL.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu, arm-none-linux-gnueabihf,
> x86_64-pc-linux-gnu -m32, -m64 and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> 	* simplify-rtx.cc (simplify_context::simplify_unary_operation): Try
> 	simplifying operand.
> 	(simplify_const_relational_operation): Simplify vector EQ and NE.
> 	(test_vector_int_const_compare): New.
> 	(test_vector_int_const_compare_ops): New.
> 	(simplify_rtx_cc_tests): Use them.
>
> ---
>
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> index a20a61c5dddbc80b23a9489d925a2c31b2163458..7e83e80246b70c81c388e77967f645d171efe983 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -886,6 +886,10 @@ simplify_context::simplify_unary_operation (rtx_code code, machine_mode mode,
>  
>    trueop = avoid_constant_pool_reference (op);
>  
> +  /* If the operand is not a reg or constant try simplifying it first.  */
> +  if (rtx tmp_op = simplify_rtx (op))
> +    op = tmp_op;
> +

We shouldn't need to do this.  The assumption is that the operands are
already simplified.

Which caller required this?

>    tem = simplify_const_unary_operation (code, mode, trueop, op_mode);
>    if (tem)
>      return tem;
> @@ -6354,6 +6358,35 @@ simplify_const_relational_operation (enum rtx_code code,
>  	return 0;
>      }
>  
> +  /* Check if the operands are a vector EQ or NE comparison.  */
> +  if (VECTOR_MODE_P (mode)
> +      && INTEGRAL_MODE_P (mode)
> +      && GET_CODE (op0) == CONST_VECTOR
> +      && GET_CODE (op1) == CONST_VECTOR
> +      && (code == EQ || code == NE))
> +    {
> +      if (rtx_equal_p (op0, op1))
> +	return code == EQ ? const_true_rtx : const0_rtx;
> +
> +      unsigned int npatterns0, npatterns1;
> +      if (CONST_VECTOR_NUNITS (op0).is_constant (&npatterns0)
> +	  && CONST_VECTOR_NUNITS (op1).is_constant (&npatterns1))
> +	{
> +	  if (npatterns0 != npatterns1)
> +	    return code == EQ ? const0_rtx : const_true_rtx;

This looks like a typing error.  The operands have to have the same
number of elements.  But...

> +
> +	  for (unsigned i = 0; i < npatterns0; i++)
> +	    {
> +	      rtx val0 = CONST_VECTOR_ELT (op0, i);
> +	      rtx val1 = CONST_VECTOR_ELT (op1, i);
> +	      if (!rtx_equal_p (val0, val1))
> +		return code == EQ ? const0_rtx : const_true_rtx;
> +	    }
> +
> +	  return code == EQ ? const_true_rtx : const0_rtx;
> +	}

...when is this loop needed?  For constant-sized vectors, isn't the
result always rtx_equal_p for EQ and !rtx_equal_p for NE?  If we have
equal vectors for which rtx_equal_p returns false then that should be
fixed.

For variable-sized vectors, I suppose the question is whether the
first unequal element is found in the minimum vector length, or whether
it only occurs for larger lengths.  In the former case we can fold at
compile time, but in the latter case we can't.

So we probably do want the loop for variable-length vectors, up to
constant_lower_bound (CONST_VECTOR_NUNITS (...)).

> +    }
> +
>    /* We can't simplify MODE_CC values since we don't know what the
>       actual comparison is.  */
>    if (GET_MODE_CLASS (GET_MODE (op0)) == MODE_CC)
> @@ -8820,6 +8853,55 @@ test_vector_ops ()
>      }
>  }
>  
> +/* Verify vector constant comparisons for EQ and NE.  */
> +
> +static void
> +test_vector_int_const_compare (machine_mode mode)
> +{
> +  rtx zeros = CONST0_RTX (mode);
> +  rtx minusone = CONSTM1_RTX (mode);
> +  rtx series_0_1 = gen_const_vec_series (mode, const0_rtx, const1_rtx);
> +  ASSERT_RTX_EQ (const0_rtx,
> +		 simplify_const_relational_operation (EQ, mode, zeros,
> +						      CONST1_RTX (mode)));
> +  ASSERT_RTX_EQ (const_true_rtx,
> +		 simplify_const_relational_operation (EQ, mode, zeros,
> +						      CONST0_RTX (mode)));
> +  ASSERT_RTX_EQ (const_true_rtx,
> +		 simplify_const_relational_operation (EQ, mode, minusone,
> +						      CONSTM1_RTX (mode)));
> +  ASSERT_RTX_EQ (const_true_rtx,
> +		 simplify_const_relational_operation (NE, mode, zeros,
> +						      CONST1_RTX (mode)));
> +  ASSERT_RTX_EQ (const_true_rtx,
> +		 simplify_const_relational_operation (NE, mode, zeros,
> +						      series_0_1));
> +  ASSERT_RTX_EQ (const0_rtx,
> +		 simplify_const_relational_operation (EQ, mode, zeros,
> +						      series_0_1));
> +}
> +
> +/* Verify some simplifications involving vectors integer comparisons.  */
> +
> +static void
> +test_vector_int_const_compare_ops ()
> +{
> +  for (unsigned int i = 0; i < NUM_MACHINE_MODES; ++i)
> +    {
> +      machine_mode mode = (machine_mode) i;
> +      if (VECTOR_MODE_P (mode)
> +	  && INTEGRAL_MODE_P (mode)
> +	  && GET_MODE_NUNITS (mode).is_constant ())
> +	{
> +	  if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT
> +	      && maybe_gt (GET_MODE_NUNITS (mode), 2))
> +	    {
> +	      test_vector_int_const_compare (mode);
> +	    }
> +	}

If the above comments are right, we should be able to do this for
variable-sized vectors as well.

Thanks,
Richard

> +    }
> +}
> +
>  template<unsigned int N>
>  struct simplify_const_poly_int_tests
>  {
> @@ -8875,6 +8957,7 @@ simplify_rtx_cc_tests ()
>  {
>    test_scalar_ops ();
>    test_vector_ops ();
> +  test_vector_int_const_compare_ops ();
>    simplify_const_poly_int_tests<NUM_POLY_INT_COEFFS>::run ();
>  }
Tamar Christina Sept. 6, 2024, 1:47 p.m. UTC | #2 
> -----Original Message-----
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Friday, September 6, 2024 2:21 PM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>
> Subject: Re: [PATCH 3/4][rtl]: simplify boolean vector EQ and NE comparisons
> 
> Tamar Christina <tamar.christina@arm.com> writes:
> > Hi All,
> >
> > This adds vector constant simplification for EQ and NE.  This is useful since
> > the vectorizer generates a lot more vector compares now, in particular NE and EQ
> > and so these help us optimize cases where the values were not known at GIMPLE
> > but instead only at RTL.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu, arm-none-linux-gnueabihf,
> > x86_64-pc-linux-gnu -m32, -m64 and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > 	* simplify-rtx.cc (simplify_context::simplify_unary_operation): Try
> > 	simplifying operand.
> > 	(simplify_const_relational_operation): Simplify vector EQ and NE.
> > 	(test_vector_int_const_compare): New.
> > 	(test_vector_int_const_compare_ops): New.
> > 	(simplify_rtx_cc_tests): Use them.
> >
> > ---
> >
> > diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> > index
> a20a61c5dddbc80b23a9489d925a2c31b2163458..7e83e80246b70c81c388e77
> 967f645d171efe983 100644
> > --- a/gcc/simplify-rtx.cc
> > +++ b/gcc/simplify-rtx.cc
> > @@ -886,6 +886,10 @@ simplify_context::simplify_unary_operation (rtx_code
> code, machine_mode mode,
> >
> >    trueop = avoid_constant_pool_reference (op);
> >
> > +  /* If the operand is not a reg or constant try simplifying it first.  */
> > +  if (rtx tmp_op = simplify_rtx (op))
> > +    op = tmp_op;
> > +
> 
> We shouldn't need to do this.  The assumption is that the operands are
> already simplified.
> 
> Which caller required this?
> 
> >    tem = simplify_const_unary_operation (code, mode, trueop, op_mode);
> >    if (tem)
> >      return tem;
> > @@ -6354,6 +6358,35 @@ simplify_const_relational_operation (enum rtx_code
> code,
> >  	return 0;
> >      }
> >
> > +  /* Check if the operands are a vector EQ or NE comparison.  */
> > +  if (VECTOR_MODE_P (mode)
> > +      && INTEGRAL_MODE_P (mode)
> > +      && GET_CODE (op0) == CONST_VECTOR
> > +      && GET_CODE (op1) == CONST_VECTOR
> > +      && (code == EQ || code == NE))
> > +    {
> > +      if (rtx_equal_p (op0, op1))
> > +	return code == EQ ? const_true_rtx : const0_rtx;
> > +
> > +      unsigned int npatterns0, npatterns1;
> > +      if (CONST_VECTOR_NUNITS (op0).is_constant (&npatterns0)
> > +	  && CONST_VECTOR_NUNITS (op1).is_constant (&npatterns1))
> > +	{
> > +	  if (npatterns0 != npatterns1)
> > +	    return code == EQ ? const0_rtx : const_true_rtx;
> 
> This looks like a typing error.  The operands have to have the same
> number of elements.  But...
> 
> > +
> > +	  for (unsigned i = 0; i < npatterns0; i++)
> > +	    {
> > +	      rtx val0 = CONST_VECTOR_ELT (op0, i);
> > +	      rtx val1 = CONST_VECTOR_ELT (op1, i);
> > +	      if (!rtx_equal_p (val0, val1))
> > +		return code == EQ ? const0_rtx : const_true_rtx;
> > +	    }
> > +
> > +	  return code == EQ ? const_true_rtx : const0_rtx;
> > +	}
> 
> ...when is this loop needed?  For constant-sized vectors, isn't the
> result always rtx_equal_p for EQ and !rtx_equal_p for NE?  If we have
> equal vectors for which rtx_equal_p returns false then that should be
> fixed.

Hmm I suppose, I guess

      if (rtx_equal_p (op0, op1))
	return code == EQ ? const_true_rtx : const0_rtx;
      else
	return code == NE ? const_true_rtx : const0_rtx;

does the same thing, 

Fair, I just didn't think about it ☹

> 
> For variable-sized vectors, I suppose the question is whether the
> first unequal element is found in the minimum vector length, or whether
> it only occurs for larger lengths.  In the former case we can fold at
> compile time, but in the latter case we can't.
> 
> So we probably do want the loop for variable-length vectors, up to
> constant_lower_bound (CONST_VECTOR_NUNITS (...)).
> 
> > +    }
> > +
> >    /* We can't simplify MODE_CC values since we don't know what the
> >       actual comparison is.  */
> >    if (GET_MODE_CLASS (GET_MODE (op0)) == MODE_CC)
> > @@ -8820,6 +8853,55 @@ test_vector_ops ()
> >      }
> >  }
> >
> > +/* Verify vector constant comparisons for EQ and NE.  */
> > +
> > +static void
> > +test_vector_int_const_compare (machine_mode mode)
> > +{
> > +  rtx zeros = CONST0_RTX (mode);
> > +  rtx minusone = CONSTM1_RTX (mode);
> > +  rtx series_0_1 = gen_const_vec_series (mode, const0_rtx, const1_rtx);
> > +  ASSERT_RTX_EQ (const0_rtx,
> > +		 simplify_const_relational_operation (EQ, mode, zeros,
> > +						      CONST1_RTX (mode)));
> > +  ASSERT_RTX_EQ (const_true_rtx,
> > +		 simplify_const_relational_operation (EQ, mode, zeros,
> > +						      CONST0_RTX (mode)));
> > +  ASSERT_RTX_EQ (const_true_rtx,
> > +		 simplify_const_relational_operation (EQ, mode, minusone,
> > +						      CONSTM1_RTX (mode)));
> > +  ASSERT_RTX_EQ (const_true_rtx,
> > +		 simplify_const_relational_operation (NE, mode, zeros,
> > +						      CONST1_RTX (mode)));
> > +  ASSERT_RTX_EQ (const_true_rtx,
> > +		 simplify_const_relational_operation (NE, mode, zeros,
> > +						      series_0_1));
> > +  ASSERT_RTX_EQ (const0_rtx,
> > +		 simplify_const_relational_operation (EQ, mode, zeros,
> > +						      series_0_1));
> > +}
> > +
> > +/* Verify some simplifications involving vectors integer comparisons.  */
> > +
> > +static void
> > +test_vector_int_const_compare_ops ()
> > +{
> > +  for (unsigned int i = 0; i < NUM_MACHINE_MODES; ++i)
> > +    {
> > +      machine_mode mode = (machine_mode) i;
> > +      if (VECTOR_MODE_P (mode)
> > +	  && INTEGRAL_MODE_P (mode)
> > +	  && GET_MODE_NUNITS (mode).is_constant ())
> > +	{
> > +	  if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT
> > +	      && maybe_gt (GET_MODE_NUNITS (mode), 2))
> > +	    {
> > +	      test_vector_int_const_compare (mode);
> > +	    }
> > +	}
> 
> If the above comments are right, we should be able to do this for
> variable-sized vectors as well.

Sure, will give it a try, thanks.

Cheers,
Tamar
> 
> Thanks,
> Richard
> 
> > +    }
> > +}
> > +
> >  template<unsigned int N>
> >  struct simplify_const_poly_int_tests
> >  {
> > @@ -8875,6 +8957,7 @@ simplify_rtx_cc_tests ()
> >  {
> >    test_scalar_ops ();
> >    test_vector_ops ();
> > +  test_vector_int_const_compare_ops ();
> >    simplify_const_poly_int_tests<NUM_POLY_INT_COEFFS>::run ();
> >  }
diff mbox series

Patch

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index a20a61c5dddbc80b23a9489d925a2c31b2163458..7e83e80246b70c81c388e77967f645d171efe983 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -886,6 +886,10 @@  simplify_context::simplify_unary_operation (rtx_code code, machine_mode mode,
 
   trueop = avoid_constant_pool_reference (op);
 
+  /* If the operand is not a reg or constant try simplifying it first.  */
+  if (rtx tmp_op = simplify_rtx (op))
+    op = tmp_op;
+
   tem = simplify_const_unary_operation (code, mode, trueop, op_mode);
   if (tem)
     return tem;
@@ -6354,6 +6358,35 @@  simplify_const_relational_operation (enum rtx_code code,
 	return 0;
     }
 
+  /* Check if the operands are a vector EQ or NE comparison.  */
+  if (VECTOR_MODE_P (mode)
+      && INTEGRAL_MODE_P (mode)
+      && GET_CODE (op0) == CONST_VECTOR
+      && GET_CODE (op1) == CONST_VECTOR
+      && (code == EQ || code == NE))
+    {
+      if (rtx_equal_p (op0, op1))
+	return code == EQ ? const_true_rtx : const0_rtx;
+
+      unsigned int npatterns0, npatterns1;
+      if (CONST_VECTOR_NUNITS (op0).is_constant (&npatterns0)
+	  && CONST_VECTOR_NUNITS (op1).is_constant (&npatterns1))
+	{
+	  if (npatterns0 != npatterns1)
+	    return code == EQ ? const0_rtx : const_true_rtx;
+
+	  for (unsigned i = 0; i < npatterns0; i++)
+	    {
+	      rtx val0 = CONST_VECTOR_ELT (op0, i);
+	      rtx val1 = CONST_VECTOR_ELT (op1, i);
+	      if (!rtx_equal_p (val0, val1))
+		return code == EQ ? const0_rtx : const_true_rtx;
+	    }
+
+	  return code == EQ ? const_true_rtx : const0_rtx;
+	}
+    }
+
   /* We can't simplify MODE_CC values since we don't know what the
      actual comparison is.  */
   if (GET_MODE_CLASS (GET_MODE (op0)) == MODE_CC)
@@ -8820,6 +8853,55 @@  test_vector_ops ()
     }
 }
 
+/* Verify vector constant comparisons for EQ and NE.  */
+
+static void
+test_vector_int_const_compare (machine_mode mode)
+{
+  rtx zeros = CONST0_RTX (mode);
+  rtx minusone = CONSTM1_RTX (mode);
+  rtx series_0_1 = gen_const_vec_series (mode, const0_rtx, const1_rtx);
+  ASSERT_RTX_EQ (const0_rtx,
+		 simplify_const_relational_operation (EQ, mode, zeros,
+						      CONST1_RTX (mode)));
+  ASSERT_RTX_EQ (const_true_rtx,
+		 simplify_const_relational_operation (EQ, mode, zeros,
+						      CONST0_RTX (mode)));
+  ASSERT_RTX_EQ (const_true_rtx,
+		 simplify_const_relational_operation (EQ, mode, minusone,
+						      CONSTM1_RTX (mode)));
+  ASSERT_RTX_EQ (const_true_rtx,
+		 simplify_const_relational_operation (NE, mode, zeros,
+						      CONST1_RTX (mode)));
+  ASSERT_RTX_EQ (const_true_rtx,
+		 simplify_const_relational_operation (NE, mode, zeros,
+						      series_0_1));
+  ASSERT_RTX_EQ (const0_rtx,
+		 simplify_const_relational_operation (EQ, mode, zeros,
+						      series_0_1));
+}
+
+/* Verify some simplifications involving vectors integer comparisons.  */
+
+static void
+test_vector_int_const_compare_ops ()
+{
+  for (unsigned int i = 0; i < NUM_MACHINE_MODES; ++i)
+    {
+      machine_mode mode = (machine_mode) i;
+      if (VECTOR_MODE_P (mode)
+	  && INTEGRAL_MODE_P (mode)
+	  && GET_MODE_NUNITS (mode).is_constant ())
+	{
+	  if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT
+	      && maybe_gt (GET_MODE_NUNITS (mode), 2))
+	    {
+	      test_vector_int_const_compare (mode);
+	    }
+	}
+    }
+}
+
 template<unsigned int N>
 struct simplify_const_poly_int_tests
 {
@@ -8875,6 +8957,7 @@  simplify_rtx_cc_tests ()
 {
   test_scalar_ops ();
   test_vector_ops ();
+  test_vector_int_const_compare_ops ();
   simplify_const_poly_int_tests<NUM_POLY_INT_COEFFS>::run ();
 }