diff mbox series

range_int_cst_p handling in extract_range_from_binary_expr_1

Message ID 87vakd1sn3.fsf@linaro.org
State New
Headers show
Series range_int_cst_p handling in extract_range_from_binary_expr_1 | expand

Commit Message

Richard Sandiford Sept. 20, 2017, 12:06 p.m. UTC
extract_range_from_binary_expr_1 had:

      if (range_int_cst_p (&vr0)
          && range_int_cst_p (&vr1)
          && TYPE_OVERFLOW_WRAPS (expr_type))
        ...
      ...
      extract_range_from_multiplicative_op_1 (vr, code, &vr0, &vr1);

but extract_range_from_multiplicative_op_1 also requires range_int_cst_p.
I think we should bail out if either range isn't a constant.

This might only be theoretical with current sources, but it's needed
once polynomial constants are added.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.
OK to install?

Richard


2017-09-20  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* tree-vrp.c (extract_range_from_binary_expr_1): Don't call
	extract_range_from_multiplicative_op_1 if !range_int_cst_p.

Comments

Richard Biener Sept. 20, 2017, 3:54 p.m. UTC | #1
On Wed, Sep 20, 2017 at 2:06 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> extract_range_from_binary_expr_1 had:

>

>       if (range_int_cst_p (&vr0)

>           && range_int_cst_p (&vr1)

>           && TYPE_OVERFLOW_WRAPS (expr_type))

>         ...

>       ...

>       extract_range_from_multiplicative_op_1 (vr, code, &vr0, &vr1);

>

> but extract_range_from_multiplicative_op_1 also requires range_int_cst_p.

> I think we should bail out if either range isn't a constant.

>

> This might only be theoretical with current sources, but it's needed

> once polynomial constants are added.

>

> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.

> OK to install?


extract_range_from_multiplicative_op_1 also allows anti-ranges but indeed
requires INTEGER_CSTs.  Note it won't get anti-ranges because we handle
this case by ranges_from_anti_range.

Thus ok if you also remove the apparent anti-range handling from
extract_range_from_multiplicative_op_1 (in the assert).

Thanks,
Richard.

> Richard

>

>

> 2017-09-20  Richard Sandiford  <richard.sandiford@linaro.org>

>             Alan Hayward  <alan.hayward@arm.com>

>             David Sherwood  <david.sherwood@arm.com>

>

> gcc/

>         * tree-vrp.c (extract_range_from_binary_expr_1): Don't call

>         extract_range_from_multiplicative_op_1 if !range_int_cst_p.

>

> Index: gcc/tree-vrp.c

> ===================================================================

> --- gcc/tree-vrp.c      2017-08-30 12:18:46.631541422 +0100

> +++ gcc/tree-vrp.c      2017-09-20 13:04:20.397027078 +0100

> @@ -2462,9 +2462,14 @@ extract_range_from_binary_expr_1 (value_

>        signop sign = TYPE_SIGN (expr_type);

>        unsigned int prec = TYPE_PRECISION (expr_type);

>

> -      if (range_int_cst_p (&vr0)

> -         && range_int_cst_p (&vr1)

> -         && TYPE_OVERFLOW_WRAPS (expr_type))

> +      if (!range_int_cst_p (&vr0)

> +         || !range_int_cst_p (&vr1))

> +       {

> +         set_value_range_to_varying (vr);

> +         return;

> +       }

> +

> +      if (TYPE_OVERFLOW_WRAPS (expr_type))

>         {

>           typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;

>           typedef generic_wide_int
Richard Sandiford Sept. 22, 2017, 5:01 p.m. UTC | #2
Richard Biener <richard.guenther@gmail.com> writes:
> On Wed, Sep 20, 2017 at 2:06 PM, Richard Sandiford

> <richard.sandiford@linaro.org> wrote:

>> extract_range_from_binary_expr_1 had:

>>

>>       if (range_int_cst_p (&vr0)

>>           && range_int_cst_p (&vr1)

>>           && TYPE_OVERFLOW_WRAPS (expr_type))

>>         ...

>>       ...

>>       extract_range_from_multiplicative_op_1 (vr, code, &vr0, &vr1);

>>

>> but extract_range_from_multiplicative_op_1 also requires range_int_cst_p.

>> I think we should bail out if either range isn't a constant.

>>

>> This might only be theoretical with current sources, but it's needed

>> once polynomial constants are added.

>>

>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linus-gnu.

>> OK to install?

>

> extract_range_from_multiplicative_op_1 also allows anti-ranges but indeed

> requires INTEGER_CSTs.  Note it won't get anti-ranges because we handle

> this case by ranges_from_anti_range.

>

> Thus ok if you also remove the apparent anti-range handling from

> extract_range_from_multiplicative_op_1 (in the assert).


Thanks, here's what I committed after testing as before.

Richard


2017-09-22  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* tree-vrp.c (extract_range_from_multiplicative_op_1): Assert
	for VR_RANGE only; don't allow VR_ANTI_RANGE.
	(extract_range_from_binary_expr_1): Don't call
	extract_range_from_multiplicative_op_1 if !range_int_cst_p.

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	2017-09-21 22:35:16.639777740 +0100
+++ gcc/tree-vrp.c	2017-09-22 17:46:45.207205032 +0100
@@ -1851,8 +1851,7 @@ extract_range_from_multiplicative_op_1 (
 	      || code == ROUND_DIV_EXPR
 	      || code == RSHIFT_EXPR
 	      || code == LSHIFT_EXPR);
-  gcc_assert ((vr0->type == VR_RANGE
-	       || (code == MULT_EXPR && vr0->type == VR_ANTI_RANGE))
+  gcc_assert (vr0->type == VR_RANGE
 	      && vr0->type == vr1->type);
 
   rtype = vr0->type;
@@ -2462,9 +2461,14 @@ extract_range_from_binary_expr_1 (value_
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
 
-      if (range_int_cst_p (&vr0)
-	  && range_int_cst_p (&vr1)
-	  && TYPE_OVERFLOW_WRAPS (expr_type))
+      if (!range_int_cst_p (&vr0)
+	  || !range_int_cst_p (&vr1))
+	{
+	  set_value_range_to_varying (vr);
+	  return;
+	}
+
+      if (TYPE_OVERFLOW_WRAPS (expr_type))
 	{
 	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
 	  typedef generic_wide_int
diff mbox series

Patch

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	2017-08-30 12:18:46.631541422 +0100
+++ gcc/tree-vrp.c	2017-09-20 13:04:20.397027078 +0100
@@ -2462,9 +2462,14 @@  extract_range_from_binary_expr_1 (value_
       signop sign = TYPE_SIGN (expr_type);
       unsigned int prec = TYPE_PRECISION (expr_type);
 
-      if (range_int_cst_p (&vr0)
-	  && range_int_cst_p (&vr1)
-	  && TYPE_OVERFLOW_WRAPS (expr_type))
+      if (!range_int_cst_p (&vr0)
+	  || !range_int_cst_p (&vr1))
+	{
+	  set_value_range_to_varying (vr);
+	  return;
+	}
+
+      if (TYPE_OVERFLOW_WRAPS (expr_type))
 	{
 	  typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int;
 	  typedef generic_wide_int