On Wed, Jun 28, 2017 at 9:56 AM, Aldy Hernandez <al...@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
