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

[Kugan <kugan.vivekanandarajah@linaro.org>]

> +  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?

> 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?

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