This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH PR69052]Check if loop inv can be propagated into mem ref with additional addr expr canonicalization
- From: "Bin.Cheng" <amker dot cheng at gmail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: Bin Cheng <Bin dot Cheng at arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Thu, 25 Feb 2016 08:47:33 +0000
- Subject: Re: [PATCH PR69052]Check if loop inv can be propagated into mem ref with additional addr expr canonicalization
- Authentication-results: sourceware.org; auth=none
- References: <HE1PR08MB050748A21B9A8BA2CEBA3D21E7D60 at HE1PR08MB0507 dot eurprd08 dot prod dot outlook dot com> <56BC34CE dot 20100 at redhat dot com> <CAHFci29GNDZXzoM75g1BRhk_B5h=2DH_2viJEEd9tbd=bnAVyg at mail dot gmail dot com> <56BD189A dot 1040409 at redhat dot com> <VI1PR08MB0512E385355AD7F24BF1497EE7AD0 at VI1PR08MB0512 dot eurprd08 dot prod dot outlook dot com> <56C7962A dot 4010508 at redhat dot com> <CAHFci2-f2HKAo-KMrk15Hm8wqw4-77pnwL5mCi0A2hP21F12sg at mail dot gmail dot com> <56CEA194 dot 1090204 at redhat dot com>
On Thu, Feb 25, 2016 at 6:39 AM, Jeff Law <law@redhat.com> wrote:
> On 02/22/2016 02:22 AM, Bin.Cheng wrote:
>
>>> My only question is why didn't you use FOR_EACH_SUBRTX_VRA from
>>> rtl-iter.h
>>> to walk the RTX expressions in collect_address_parts and
>>> canonicalize_address_mult?
>>
>> Hi Jeff,
>> Nothing special, just I haven't used this before, also
>> canonicalize_address_mult is alphabetically copied from fwprop.c. One
>> question is when rewriting SHIFT to MULT, we need to modify rtl
>> expression in place, does FOR_EACH_SUBRTX iterator support this? If
>> yes, what is the behavior for modified sub-expression?
>
> Hmm. The question of semantics when we change the underlying
> sub-expressions is an interesting one.
>
> While I think we're OK in practice using the iterators, I think that's more
> of an accident than by design -- if MULT/ASHIFT had a different underlying
> RTL structure then I'm much less sure using the iterators would be safe.
Hi Jeff,
Yes, I thought about this too and finally decided to skip sub rtxes in
modified MULT/ASHIFT expressions. I think this will make the patch
with FOR_EACH_SUBRTX iterator safe. Although it doesn't dive into
MULT/ASHIFT while the original one does, I don't think there is such
case in practice, after all we can't handle such complicated address
expression either.
>
> Let's go with your original patch that didn't use the iterators. Sorry for
> making you do the additional work/testing to build the iterator version.
Not even a problem.
> But after pondering the issue you raised, I think your original patch is
> safer.
So in conclusion, I think both versions should be safe, the iterator
one is definitely cleaner. It is your call which version I should
apply.
Thanks,
bin
>
>
> jeff