On Wed, Nov 6, 2013 at 6:06 PM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
> 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;
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.
Richard.
>
>> + {
>> + 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
