Re: [PATCH]Fix computation of offset in ivopt

["Bin.Cheng" <amker dot cheng at gmail dot com>]

On Thu, Nov 7, 2013 at 1:06 AM, 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:
Hi,

Sorry for this, I should have handled the bsizetype in the first
place.  What I am not sure is how to do with big DECL_FIELD_BIT_OFFSET
which even can't be accepted by host_integerp.  Generally it's very
unlikely to happen and we don't handle such exceptions in GCC, right?
I will send a patch fixing the issue.

Hopefully, with the patch just got approved at
http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02367.html , IVOPT will
never see complex address expressions any more, and major part of
strip_offset_1 (maybe other functions) can be cleaned.

Thanks,
bin

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



-- 
Best Regards.