On Thu, Sep 19, 2013 at 7:07 AM, Kugan
<kugan.vivekanandara...@linaro.org> wrote:
> Thanks Richard for the review.
>
> On 18/09/13 18:55, Richard Biener wrote:
>>
>> On Wed, 18 Sep 2013, Kugan wrote:
>>
>>>
>>> Thanks Richard for the review.
>>> On 16/09/13 23:43, Richard Biener wrote:
>>>>
>>>> On Mon, 16 Sep 2013, Kugan wrote:
>>>>
>
> [Snip]
>
>
>>>
>>> +2013-09-17 Kugan Vivekanandarajah <kug...@linaro.org>
>>> +
>>> + * gimple-pretty-print.c (dump_ssaname_info) : New function.
>>> + * gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
>>> + * (pp_gimple_stmt_1) : Likewise.
>>
>>
>> ChangeLog should be formated
>>
>> * gimple-pretty-print.c (dump_ssaname_info): New function.
>> (dump_gimple_phi): Call it.
>> (pp_gimple_stmt_1: Likewise.
>> * tree-pretty-print.c (dump_intger_cst_node): New function.
>> ...
>>
>>
>> + pp_printf (buffer, "# RANGE ");
>> + pp_printf (buffer, "%s[", range_type == VR_RANGE ? "" : "~");
>> + dump_intger_cst_node (buffer,
>> + double_int_to_tree (TREE_TYPE (node),
>> min));
>>
>> I was asking for a pp_double_int, not a dump_integer_cst_node function
>> as now you are creating a tree node in GC memory just to dump its
>> contents ... pp_double_int needs to be passed information on the
>> signedness of the value. It would roughly look like
>>
>
> Sorry, I understood it wrong.
>
>
>> pp_double_int (pretty_printer *pp, double_int d, bool uns)
>> {
>> if (d.fits_shwi ())
>> pp_wide_integer (pp, d.low);
>> else if (d.fits_uhwi ())
>> pp_unsigned_wide_integer (pp, d.low);
>> else
>> {
>> unsigned HOST_WIDE_INT low = d.low;
>> HOST_WIDE_INT high = d.high;
>> if (!uns && d.is_negative ())
>> {
>> pp_minus (pp);
>> high = ~high + !low;
>> low = -low;
>> }
>> /* Would "%x%0*x" or "%x%*0x" get zero-padding on all
>> systems? */
>> sprintf (pp_buffer (pp)->digit_buffer,
>> HOST_WIDE_INT_PRINT_DOUBLE_HEX,
>> (unsigned HOST_WIDE_INT) high, low);
>> pp_string (pp, pp_buffer (pp)->digit_buffer);
>> }
>> }
>>
>> and the INTEGER_CST case would use it like
>>
>> if (TREE_CODE (TREE_TYPE (node)) == POINTER_TYPE)
>> ...
>> else
>> pp_double_int (buffer, tree_to_double_int (node),
>> TYPE_UNSIGNED (TREE_TYPE (node)));
>>
>>
>> +enum value_range_type
>> +get_range_info (tree name, double_int *min, double_int *max)
>> +{
>>
>> ah, I see you have already made an appropriate change here.
>>
>> + /* Check for an empty range with minimum zero (of type
>> + unsigned) that will wraparround. */
>> + if (!(TYPE_UNSIGNED (TREE_TYPE (name))
>> + && integer_zerop (vr_value[i]->min)))
>> + set_range_info (name,
>> + tree_to_double_int (vr_value[i]->max)
>> + + double_int_one,
>> + tree_to_double_int (vr_value[i]->min)
>> + - double_int_one);
>>
>> Yeah, I think ~[0,0] is the only anti-range that can be represented as
>> range that we keep. So maybe
>>
>> if (TYPE_UNSIGNED (TREE_TYPE (name))
>> && integer_zerop (vr_value[i]->min)
>> && integer_zerop (vr_value[i]->max))
>> set_range_info (name,
>> double_int_one,
>> double_int::max_value
>> (TYPE_PRECISION (TREE_TYPE (name)),
>> true));
>> else
>> set_range_info (name,
>> + tree_to_double_int (vr_value[i]->max)
>> + + double_int_one,
>> + tree_to_double_int (vr_value[i]->min)
>> + - double_int_one);
>>
>> to preserve ~[0,0] which looks like an important case when for example
>> looking at a divisor in a division.
>>
>> Ok with those changes.
>>
>
> I have changed all of the above in the attached patch and ChangeLog. If this
> is OK, could someone please commit it for me. I don’t have access to commit
> it.
>
> Bootstrapped and regtested on x86_64-unknown-linux-gnu and arm-none
> linux-gnueabi.
Ok.
Thanks and sorry that it took so long,
Richard.
> Thanks,
> Kugan
>
>
