[PATCH]Fix computation of offset in ivopt

[Richard Biener]

On Wed, Nov 13, 2013 at 12:18 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Thu, Nov 7, 2013 at 6:47 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Wed, Nov 6, 2013 at 6:06 PM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Hi,
>>>
>>> "bin.cheng" <bin.cheng@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;
>>
>> I think that's simply overly restrictive on the side of cst_and_fits_in_hwi.
>> I suppose this function is meant to complement int_cst_value (digging
>> in history might be nice here).
>>
>>> Should this be host_integerp (DECL_FIELD_BIT_OFFSET (field), 0) instead?
>>
>> That's not the same as it rejects -1U.  The function seems to ask
>> if the value, if casted to unsigned fits in a HOST_WIDE_INT.
>>
>> So just drop the precision check from cst_and_fits_in_hwi.
> Hi,
> With check in of patch lowering address expression in ivo, that piece
> of code won't be executed anymore.  So still need to drop precision
> check in cst_and_fits_in_hwi?  The major part of strip_offset_1 can be
> cleaned now.

I think it's a good cleanup, so if you can bootstrap & test that
change separately....?

Thanks,
Richard.

> Thanks,
> bin
> --
> Best Regards.