diff mbox

[3/7] Optimize ZEXT_EXPR with tree-vrp

Message ID 5615AD60.6040904@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah Oct. 7, 2015, 11:40 p.m. UTC
On 07/10/15 19:20, Richard Biener wrote:
> On Wed, Oct 7, 2015 at 1:12 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>
>> Hi Richard,
>>
>> Thanks for the review.
>>
>> On 15/09/15 23:08, Richard Biener wrote:
>>>
>>> On Mon, Sep 7, 2015 at 4:58 AM, Kugan <kugan.vivekanandarajah@linaro.org>
>>> wrote:
>>>>
>>>> This patch tree-vrp handling and optimization for ZEXT_EXPR.
>>>
>>>
>>> +  else if (code == SEXT_EXPR)
>>> +    {
>>> +      gcc_assert (range_int_cst_p (&vr1));
>>> +      unsigned int prec = tree_to_uhwi (vr1.min);
>>> +      type = vr0.type;
>>> +      wide_int tmin, tmax;
>>> +      wide_int type_min, type_max;
>>> +      wide_int may_be_nonzero, must_be_nonzero;
>>> +
>>> +      gcc_assert (!TYPE_UNSIGNED (expr_type));
>>>
>>> hmm, I don't think we should restrict SEXT_EXPR this way.  SEXT_EXPR
>>> should operate on both signed and unsigned types and the result type
>>> should be the same as the type of operand 0.
>>>
>>> +      type_min = wi::shwi (1 << (prec - 1),
>>> +                          TYPE_PRECISION (TREE_TYPE (vr0.min)));
>>> +      type_max = wi::shwi (((1 << (prec - 1)) - 1),
>>> +                          TYPE_PRECISION (TREE_TYPE (vr0.max)));
>>>
>>> there is wi::min_value and max_value for this.
>>
>>
>> As of now, SEXT_EXPR in gimple is of the form: x = y sext 8 and types of all
>> the operand and results are of the wider type. Therefore we cant use the
>> wi::min_value. Or do you want to convert this precision (in this case 8) to
>> a type and use wi::min_value?
> 
> I don't understand - wi::min/max_value get a precision and sign, not a type.
> your 1 << (prec - 1) is even wrong for prec > 32 (it's an integer type
> expression).
> Thus
> 
>   type_min = wi::min_value (prec, SIGNED);
>   type_max = wi::max_value (prec, SIGNED);
> 

Thanks for the comments. Is the attached patch looks better. It is based
on the above. I am still assuming the position of sign-bit in SEXT_EXPR
will be less than 64bit (for calculating sign_bit in wide_int format). I
think this will always be the case but please let me know if this is not OK.

Thanks,
Kugan


> 
>> Please find the patch that addresses the other comments.

Comments

Richard Biener Oct. 9, 2015, 10:29 a.m. UTC | #1
On Thu, Oct 8, 2015 at 1:40 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>
>
> On 07/10/15 19:20, Richard Biener wrote:
>> On Wed, Oct 7, 2015 at 1:12 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>>
>>> Hi Richard,
>>>
>>> Thanks for the review.
>>>
>>> On 15/09/15 23:08, Richard Biener wrote:
>>>>
>>>> On Mon, Sep 7, 2015 at 4:58 AM, Kugan <kugan.vivekanandarajah@linaro.org>
>>>> wrote:
>>>>>
>>>>> This patch tree-vrp handling and optimization for ZEXT_EXPR.
>>>>
>>>>
>>>> +  else if (code == SEXT_EXPR)
>>>> +    {
>>>> +      gcc_assert (range_int_cst_p (&vr1));
>>>> +      unsigned int prec = tree_to_uhwi (vr1.min);
>>>> +      type = vr0.type;
>>>> +      wide_int tmin, tmax;
>>>> +      wide_int type_min, type_max;
>>>> +      wide_int may_be_nonzero, must_be_nonzero;
>>>> +
>>>> +      gcc_assert (!TYPE_UNSIGNED (expr_type));
>>>>
>>>> hmm, I don't think we should restrict SEXT_EXPR this way.  SEXT_EXPR
>>>> should operate on both signed and unsigned types and the result type
>>>> should be the same as the type of operand 0.
>>>>
>>>> +      type_min = wi::shwi (1 << (prec - 1),
>>>> +                          TYPE_PRECISION (TREE_TYPE (vr0.min)));
>>>> +      type_max = wi::shwi (((1 << (prec - 1)) - 1),
>>>> +                          TYPE_PRECISION (TREE_TYPE (vr0.max)));
>>>>
>>>> there is wi::min_value and max_value for this.
>>>
>>>
>>> As of now, SEXT_EXPR in gimple is of the form: x = y sext 8 and types of all
>>> the operand and results are of the wider type. Therefore we cant use the
>>> wi::min_value. Or do you want to convert this precision (in this case 8) to
>>> a type and use wi::min_value?
>>
>> I don't understand - wi::min/max_value get a precision and sign, not a type.
>> your 1 << (prec - 1) is even wrong for prec > 32 (it's an integer type
>> expression).
>> Thus
>>
>>   type_min = wi::min_value (prec, SIGNED);
>>   type_max = wi::max_value (prec, SIGNED);
>>
>
> Thanks for the comments. Is the attached patch looks better. It is based
> on the above. I am still assuming the position of sign-bit in SEXT_EXPR
> will be less than 64bit (for calculating sign_bit in wide_int format). I
> think this will always be the case but please let me know if this is not OK.

+      unsigned int prec = tree_to_uhwi (vr1.min);

this should use unsigned HOST_WIDE_INT

+      wide_int sign_bit = wi::shwi (1ULL << (prec - 1),
+                                   TYPE_PRECISION (TREE_TYPE (vr0.min)));

use wi::one (TYPE_PRECISION (TREE_TYPE (vr0.min))) << (prec - 1);

That is, you really need to handle precisions bigger than HOST_WIDE_INT.

But I suppose wide_int really misses a test_bit function (it has a set_bit
one already).

+         if (wi::bit_and (must_be_nonzero, sign_bit) == sign_bit)
+           {
+             /* If to-be-extended sign bit is one.  */
+             tmin = type_min;
+             tmax = may_be_nonzero;

I think tmax should be zero-extended may_be_nonzero from prec.

+         else if (wi::bit_and (may_be_nonzero, sign_bit)
+                  != sign_bit)
+           {
+             /* If to-be-extended sign bit is zero.  */
+             tmin = must_be_nonzero;
+             tmax = may_be_nonzero;

likewise here tmin/tmax should be zero-extended may/must_be_nonzero from prec.

+    case SEXT_EXPR:
+       {
+         unsigned int prec = tree_to_uhwi (op1);
+         wide_int sign_bit = wi::shwi (1ULL << (prec - 1),
+                                       TYPE_PRECISION (TREE_TYPE (vr0.min)));
+         wide_int mask = wi::shwi (((1ULL << (prec - 1)) - 1),
+                                   TYPE_PRECISION (TREE_TYPE (vr0.max)));

this has the same host precision issues of 1ULL (HOST_WIDE_INT).
There is wi::mask, eventually you can use wi::set_bit_in_zero to
produce the sign-bit wide_int (also above).

The rest of the patch looks ok.

Richard.


> Thanks,
> Kugan
>
>
>>
>>> Please find the patch that addresses the other comments.
diff mbox

Patch

From 963e5ed4576bd7f82e83b21f35c58e9962dbbc74 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Mon, 17 Aug 2015 13:45:52 +1000
Subject: [PATCH 3/7] Optimize ZEXT_EXPR with tree-vrp

---
 gcc/tree-vrp.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 2cd71a2..ada1c9f 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -2317,6 +2317,7 @@  extract_range_from_binary_expr_1 (value_range_t *vr,
       && code != LSHIFT_EXPR
       && code != MIN_EXPR
       && code != MAX_EXPR
+      && code != SEXT_EXPR
       && code != BIT_AND_EXPR
       && code != BIT_IOR_EXPR
       && code != BIT_XOR_EXPR)
@@ -2877,6 +2878,51 @@  extract_range_from_binary_expr_1 (value_range_t *vr,
       extract_range_from_multiplicative_op_1 (vr, code, &vr0, &vr1);
       return;
     }
+  else if (code == SEXT_EXPR)
+    {
+      gcc_assert (range_int_cst_p (&vr1));
+      unsigned int prec = tree_to_uhwi (vr1.min);
+      type = vr0.type;
+      wide_int tmin, tmax;
+      wide_int may_be_nonzero, must_be_nonzero;
+
+      wide_int type_min = wi::min_value (prec, SIGNED);
+      wide_int type_max = wi::max_value (prec, SIGNED);
+      type_min = wide_int_to_tree (expr_type, type_min);
+      type_max = wide_int_to_tree (expr_type, type_max);
+      wide_int sign_bit = wi::shwi (1ULL << (prec - 1),
+				    TYPE_PRECISION (TREE_TYPE (vr0.min)));
+      if (zero_nonzero_bits_from_vr (expr_type, &vr0,
+				     &may_be_nonzero,
+				     &must_be_nonzero))
+	{
+	  if (wi::bit_and (must_be_nonzero, sign_bit) == sign_bit)
+	    {
+	      /* If to-be-extended sign bit is one.  */
+	      tmin = type_min;
+	      tmax = may_be_nonzero;
+	    }
+	  else if (wi::bit_and (may_be_nonzero, sign_bit)
+		   != sign_bit)
+	    {
+	      /* If to-be-extended sign bit is zero.  */
+	      tmin = must_be_nonzero;
+	      tmax = may_be_nonzero;
+	    }
+	  else
+	    {
+	      tmin = type_min;
+	      tmax = type_max;
+	    }
+	}
+      else
+	{
+	  tmin = type_min;
+	  tmax = type_max;
+	}
+      min = wide_int_to_tree (expr_type, tmin);
+      max = wide_int_to_tree (expr_type, tmax);
+    }
   else if (code == RSHIFT_EXPR
 	   || code == LSHIFT_EXPR)
     {
@@ -9244,6 +9290,28 @@  simplify_bit_ops_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
 	  break;
 	}
       break;
+    case SEXT_EXPR:
+	{
+	  unsigned int prec = tree_to_uhwi (op1);
+	  wide_int sign_bit = wi::shwi (1ULL << (prec - 1),
+					TYPE_PRECISION (TREE_TYPE (vr0.min)));
+	  wide_int mask = wi::shwi (((1ULL << (prec - 1)) - 1),
+				    TYPE_PRECISION (TREE_TYPE (vr0.max)));
+	  mask = wi::bit_not (mask);
+	  if (wi::bit_and (must_be_nonzero0, sign_bit) == sign_bit)
+	    {
+	      /* If to-be-extended sign bit is one.  */
+	      if (wi::bit_and (must_be_nonzero0, mask) == mask)
+		op = op0;
+	    }
+	  else if (wi::bit_and (may_be_nonzero0, sign_bit) != sign_bit)
+	    {
+	      /* If to-be-extended sign bit is zero.  */
+	      if (wi::bit_and (may_be_nonzero0, mask) == 0)
+		op = op0;
+	    }
+	}
+      break;
     default:
       gcc_unreachable ();
     }
@@ -9946,6 +10014,7 @@  simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
 
 	case BIT_AND_EXPR:
 	case BIT_IOR_EXPR:
+	case SEXT_EXPR:
 	  /* Optimize away BIT_AND_EXPR and BIT_IOR_EXPR
 	     if all the bits being cleared are already cleared or
 	     all the bits being set are already set.  */
-- 
1.9.1