This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH]Fix computation of offset in ivopt
- From: "Bin.Cheng" <amker dot cheng at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: "bin.cheng" <bin dot cheng at arm dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Wed, 13 Nov 2013 19:46:32 +0800
- Subject: Re: [PATCH]Fix computation of offset in ivopt
- Authentication-results: sourceware.org; auth=none
- References: <001501ceb906$56da1c00$048e5400$ at arm dot com> <CAFiYyc0WO3M8x-0eyN4xpami9yemcX0cVy=awfOFYYmeo+OSyg at mail dot gmail dot com> <002701cebb3f$6fcd6ab0$4f684010$ at arm dot com> <CAFiYyc0M_7mSxED5YK7WYy+7mxd3Axdzdy=smnSEQj2iPx9m3A at mail dot gmail dot com> <002901cebd9f$5f7cb0a0$1e7611e0$ at arm dot com> <CAFiYyc1J1FBhsgsj9hgcMWg4o8M7soY3h4CGQWa+FyVqrEv_eQ at mail dot gmail dot com> <CAHFci295kKk7CXiEPfEGiPDy+ekew7TgEWygr-_prYbV9Sscow at mail dot gmail dot com> <CAFiYyc1r030B6H_XLt4iiLSabiCxWqN34BmQrcX+ZCfTtpMxpA at mail dot gmail dot com> <002e01cecafd$03809c60$0a81d520$ at arm dot com> <87txfpqx1p dot fsf at talisman dot default> <CAFiYyc2YkcYE=Y8A8zncTduRLaGkESQMJYrCrf08DYCmw9T+tw at mail dot gmail dot com> <CAHFci29J7m+K4gYw9GJxQP_oLs-KTssu77UkgeMwT6Ti6=GMRQ at mail dot gmail dot com> <CAFiYyc1dpFZeU9z1WqzxG77u_2H=UZUTBShKf0V=X6YOBPLRJw at mail dot gmail dot com>
On Wed, Nov 13, 2013 at 7:26 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> 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....?
Will do.
--
Best Regards.