This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL


On Thu, Sep 19, 2013 at 7:07 AM, Kugan
<kugan.vivekanandarajah@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  <kuganv@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
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]