Re: [PATCH 2/2] Enable elimination of zext/sext

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Aug 1, 2014 at 6:51 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>> +  lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
>> ...
>> +      && ((!lhs_uns && !wi::neg_p (min, TYPE_SIGN (lhs_type)))
>> ...
>> +  type_min = wide_int::from (TYPE_MIN_VALUE (lhs_type), prec,
>> +                            TYPE_SIGN (TREE_TYPE (ssa)));
>> +  type_max = wide_int::from (TYPE_MAX_VALUE (lhs_type), prec,
>> +                            TYPE_SIGN (TREE_TYPE (ssa)));
>>
>> you shouldn't try getting at lhs_type.  Btw, do you want to constrain
>> lhs_mode to MODE_INTs somewhere?
>
> Is this in addition to !INTEGRAL_TYPE_P (TREE_TYPE (ssa)))? Do you mean
> that I should check lhs_mode as well?

No, that's probably enough.

>> For TYPE_SIGN use lhs_uns instead, for the min/max value you
>> should use wi::min_value () and wi::max_value () instead.
>>
>> You are still using TYPE_SIGN (TREE_TYPE (ssa)) here and later,
>> but we computed rhs_uns "properly" using PROMOTE_MODE.
>> I think  the code with re-setting lhs_uns if rhs_uns != lhs_uns
>> and later using TYPE_SIGN again is pretty hard to follow.
>>
>> Btw, it seems you need to conditionalize the call to PROMOTE_MODE
>> on its availability.
>>
>> Isn't it simply about choosing a proper range we need to restrict
>> ssa to?  That is, dependent on rhs_uns computed by PROMOTE_MODE,
>> simply:
>>
>> +  mode = TYPE_MODE (TREE_TYPE (ssa));
>> +  rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
>> #ifdef PROMOTE_MODE
>> +  PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
>> #endif
>>
>>  if (rhs_uns)
>>    return wi::ge_p (min, 0);  // if min >= 0 then range contains positive values
>>  else
>>    return wi::le_p (max, wi::max_value (TYPE_PRECISION (TREE_TYPE
>> (ssa)), SIGNED);  // if max <= signed-max-of-type then range doesn't
>> need sign-extension
>
> I think we will have to check that ssa has necessary sign/zero extension
> when assigned to lhs_type. If PROMOTE_MODE tells us that ssa's type will
> be interpreted differently, the value range of ssa also will have
> corresponding range.  In this cases, shouldnât we have to check for
> upper and lower limit for both min and max?

Hmm?  That's exactly what the check is testing...  we know that
min <= max thus if min >= 0 then max >= 0.

zero_extension will never do anything on [0, INF]

If max < MAX-SIGNED then sign-extension will not do anything.  Ok,
sign-extension will do sth for negative values still.  So rather

  if (rhs_uns)
    return wi::geu_p (min, 0);
  else
    return wi::ges_p (min, 0) && wi::les_p (max, wi::max_value
(TYPE_PRECISION (TREE_TYPE (ssa)), SIGNED));

?

I don't like the use of int_fits_type_p you propose.

Richard.

> How about this?
>
> bool
> promoted_for_type_p (tree ssa, enum machine_mode lhs_mode, bool lhs_uns)
> {
>   wide_int min, max;
>   tree lhs_type, rhs_type;
>   bool rhs_uns;
>   enum machine_mode rhs_mode;
>   tree min_tree, max_tree;
>
>   if (ssa == NULL_TREE
>       || TREE_CODE (ssa) != SSA_NAME
>       || !INTEGRAL_TYPE_P (TREE_TYPE (ssa)))
>     return false;
>
>   /* Return FALSE if value_range is not recorded for SSA.  */
>   if (get_range_info (ssa, &min, &max) != VR_RANGE)
>     return false;
>
>   rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
>   if (rhs_uns != lhs_uns)
>     {
>       /* Signedness of LHS and RHS differs and values also cannot be
>          represented in LHS range.  */
>       unsigned int prec = min.get_precision ();
>       if ((lhs_uns && wi::neg_p (min, rhs_uns ? UNSIGNED : SIGNED))
>           || (!lhs_uns && !wi::le_p (max,
>                                     wi::max_value (prec, SIGNED),
>                                     rhs_uns ? UNSIGNED : SIGNED)))
>         return false;
>     }
>
>   /* In some architectures, modes are promoted and sign changed with
>      target defined PROMOTE_MODE macro.  If PROMOTE_MODE tells you to
>      promote _not_ according to ssa's sign then honour that.  */
>   rhs_mode = TYPE_MODE (TREE_TYPE (ssa));
> #ifdef PROMOTE_MODE
>   PROMOTE_MODE (rhs_mode, rhs_uns, TREE_TYPE (ssa));
> #endif
>
>   rhs_type = lang_hooks.types.type_for_mode (rhs_mode, rhs_uns);
>   lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
>   min_tree = wide_int_to_tree (rhs_type, min);
>   max_tree = wide_int_to_tree (rhs_type, max);
>
>   /* Check if values lies in-between the type range.  */
>   if (int_fits_type_p (min_tree, lhs_type)
>       && int_fits_type_p (max_tree, lhs_type))
>     return true;
>   else
>     return false;
> }
>
>
> Thanks,
> Kugan