Re: Avoid generating useless range info

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Jun 28, 2017 at 9:56 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
> On 06/27/2017 06:38 AM, Jakub Jelinek wrote:
>>
>> On Tue, Jun 27, 2017 at 06:26:46AM -0400, Aldy Hernandez wrote:
>>>
>>> How about this?
>>
>>
>> @@ -360,6 +363,22 @@ set_range_info (tree name, enum value_range_type
>> range_type,
>>       }
>>   }
>>   +/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name
>> +   NAME while making sure we don't store useless range info.  */
>> +
>> +void
>> +set_range_info (tree name, enum value_range_type range_type,
>> +               const wide_int_ref &min, const wide_int_ref &max)
>> +{
>> +  /* A range of the entire domain is really no range at all.  */
>> +  tree type = TREE_TYPE (name);
>> +  if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type))
>> +      && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type)))
>> +    return;
>> +
>> +  set_range_info_raw (name, range_type, min, max);
>> +}
>> +
>>
>> Won't this misbehave if we have a narrower range on some SSA_NAME and
>> call set_range_info to make it VARYING?
>> In that case (i.e. SSA_NAME_RANGE_INFO (name) != NULL), we should either
>> set_range_info_raw too (if nonzero_bits is not all ones) or clear
>> SSA_NAME_RANGE_INFO (otherwise).
>
>
> Good point.  Fixed.
>
>>     /* Gets range information MIN, MAX and returns enum value_range_type
>>      corresponding to tree ssa_name NAME.  enum value_range_type returned
>> @@ -419,9 +438,13 @@ set_nonzero_bits (tree name, const wide_int_ref
>> &mask)
>>   {
>>     gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
>>     if (SSA_NAME_RANGE_INFO (name) == NULL)
>> -    set_range_info (name, VR_RANGE,
>> -                   TYPE_MIN_VALUE (TREE_TYPE (name)),
>> -                   TYPE_MAX_VALUE (TREE_TYPE (name)));
>> +    {
>> +      if (mask == -1)
>> +       return;
>> +      set_range_info_raw (name, VR_RANGE,
>> +                         TYPE_MIN_VALUE (TREE_TYPE (name)),
>> +                         TYPE_MAX_VALUE (TREE_TYPE (name)));
>> +    }
>>     range_info_def *ri = SSA_NAME_RANGE_INFO (name);
>>     ri->set_nonzero_bits (mask);
>>
>> Similarly, if SSA_NAME_RANGE_INFO is previously non-NULL, but min/max
>> are VARYING and the new mask is -1, shouldn't we free it rather than
>> set it to the default?
>
>
> Here, if SSA_NAME_RANGE_INFO is previously non-NULL then we proceed as
> always-- just set the nonzero bits to whatever was specified (without
> clearning SSA_NAME_RANGE_INFO).  A mask of -1 and an SSA_NAME_RANGE_INFO of
> non-NULL can coexist just fine.
>
> How about this?

Ok.

Thanks,
Richard.

> Aldy