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.