Re: [PATCH]Fix computation of offset in ivopt

[Richard Sandiford]

Hi,

"bin.cheng" <bin.ch...@arm.com> writes:
> Index: gcc/tree-ssa-loop-ivopts.c
> ===================================================================
> --- gcc/tree-ssa-loop-ivopts.c        (revision 203267)
> +++ gcc/tree-ssa-loop-ivopts.c        (working copy)
> @@ -2037,12 +2037,12 @@ find_interesting_uses (struct ivopts_data *data)
>  
>  static tree
>  strip_offset_1 (tree expr, bool inside_addr, bool top_compref,
> -             unsigned HOST_WIDE_INT *offset)
> +             HOST_WIDE_INT *offset)
>  {
>    tree op0 = NULL_TREE, op1 = NULL_TREE, tmp, step;
>    enum tree_code code;
>    tree type, orig_type = TREE_TYPE (expr);
> -  unsigned HOST_WIDE_INT off0, off1, st;
> +  HOST_WIDE_INT off0, off1, st;
>    tree orig_expr = expr;
>  
>    STRIP_NOPS (expr);
> @@ -2133,19 +2133,32 @@ strip_offset_1 (tree expr, bool inside_addr, bool
>        break;
>  
>      case COMPONENT_REF:
> -      if (!inside_addr)
> -     return orig_expr;
> +      {
> +     tree field;
>  
> -      tmp = component_ref_field_offset (expr);
> -      if (top_compref
> -       && cst_and_fits_in_hwi (tmp))
> -     {
> -       /* Strip the component reference completely.  */
> -       op0 = TREE_OPERAND (expr, 0);
> -       op0 = strip_offset_1 (op0, inside_addr, top_compref, &off0);
> -       *offset = off0 + int_cst_value (tmp);
> -       return op0;
> -     }
> +     if (!inside_addr)
> +       return orig_expr;
> +
> +     tmp = component_ref_field_offset (expr);
> +     field = TREE_OPERAND (expr, 1);
> +     if (top_compref
> +         && cst_and_fits_in_hwi (tmp)
> +         && cst_and_fits_in_hwi (DECL_FIELD_BIT_OFFSET (field)))

While comparing output for wide-int and mainline, I noticed that
this condition is now always false on x86_64, since DECL_FIELD_BIT_OFFSET
is a 128-bit bitsizetype and since cst_and_fits_in_hwi rejects constants
with precision greater than HWI:

  if (TREE_CODE (x) != INTEGER_CST)
    return false;

  if (TYPE_PRECISION (TREE_TYPE (x)) > HOST_BITS_PER_WIDE_INT)
    return false;

Should this be host_integerp (DECL_FIELD_BIT_OFFSET (field), 0) instead?

> +       {
> +         HOST_WIDE_INT boffset, abs_off;
> +
> +         /* Strip the component reference completely.  */
> +         op0 = TREE_OPERAND (expr, 0);
> +         op0 = strip_offset_1 (op0, inside_addr, top_compref, &off0);
> +         boffset = int_cst_value (DECL_FIELD_BIT_OFFSET (field));
> +         abs_off = abs_hwi (boffset) / BITS_PER_UNIT;
> +         if (boffset < 0)
> +           abs_off = -abs_off;
> +
> +         *offset = off0 + int_cst_value (tmp) + abs_off;
> +         return op0;
> +       }
> +      }
>        break;

Thanks,
Richard