This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH GCC8][07/33]Offset validity check in address expression
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: "Bin.Cheng" <amker dot cheng at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 5 May 2017 09:20:20 +0200
- Subject: Re: [PATCH GCC8][07/33]Offset validity check in address expression
- Authentication-results: sourceware.org; auth=none
- References: <VI1PR0802MB217626BE7910E0234A979B3AE7190@VI1PR0802MB2176.eurprd08.prod.outlook.com> <CAFiYyc3oebX7PfXM4GRkJJYbaVK1f0+y__cBgneYozWkTDLu0A@mail.gmail.com> <CAHFci2_L2UJ4g75SaCbXGaFKHXU9qrabM=yRrQrMG8qTZ4hzzg@mail.gmail.com> <CAFiYyc1z78bcAGp0QPjGm72U1vMfJkkArDqxve6tXy-B8r3DQw@mail.gmail.com> <CAHFci28Y-Cc+gvNWi+2rmj_hWUjPLGAy9cDgsGbvcUEn=7mHRQ@mail.gmail.com>
On Thu, May 4, 2017 at 5:17 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, May 3, 2017 at 10:49 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Tue, May 2, 2017 at 7:06 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>> On Mon, Apr 24, 2017 at 11:34 AM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> On Tue, Apr 18, 2017 at 12:41 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>>>>> Hi,
>>>>> For now, we check validity of offset by computing the maximum offset then checking if
>>>>> offset is smaller than the max offset. This is inaccurate, for example, some targets
>>>>> may require offset to be aligned by power of 2. This patch introduces new interface
>>>>> checking validity of offset. It also buffers rtx among different calls.
>>>>>
>>>>> Is it OK?
>>>>
>>>> - static vec<HOST_WIDE_INT> max_offset_list;
>>>> -
>>>> + auto_vec<rtx> addr_list;
>>>> as = TYPE_ADDR_SPACE (TREE_TYPE (use->iv->base));
>>>> mem_mode = TYPE_MODE (TREE_TYPE (*use->op_p));
>>>>
>>>> - num = max_offset_list.length ();
>>>> + num = addr_list.length ();
>>>> list_index = (unsigned) as * MAX_MACHINE_MODE + (unsigned) mem_mode;
>>>> if (list_index >= num)
>>>>
>>>> num here is always zero and thus the compare is always true.
>>>>
>>>> + addr_list.safe_grow_cleared (list_index + MAX_MACHINE_MODE);
>>>> + for (; num < addr_list.length (); num++)
>>>> + addr_list[num] = NULL;
>>>>
>>>> the loop is now redundant (safe_grow_cleared)
>>>>
>>>> + addr = addr_list[list_index];
>>>> + if (!addr)
>>>> {
>>>>
>>>> always true again...
>>>>
>>>> I wonder if you really indented to drop 'static' from addr_list?
>>>> There's no caching
>>>> across function calls.
>>> Right, the redundancy is because I tried to cache across function
>>> calls with declarations like:
>>> static unsigned num = 0;
>>> static GTY ((skip)) rtx *addr_list = NULL;
>>> But this doesn't work, the addr_list[list_index] still gets corrupted somehow.
>>
>> Well, you need GTY (()), not GTY((skip)) on them. Not sure if it works
>> for function-scope decls, you have to check. Look at whether a GC
>> root is created for the variable in gt-tree-ssa-loop-ivopts.h (need to tweak
>> GTFILES in the makefile plus include that generated file). tree-ssa-address.c
>> uses a global root for mem_addr_template_list for example.
> Thanks for helping, patch updated.
> Bootstrap and test on x86_64. Is it OK?
Yes.
Thanks,
Richard.
> Thanks,
> bin
>
> 2017-05-02 Bin Cheng <bin.cheng@arm.com>
>
> * Makefile.in (GTFILES): Add tree-ssa-loop-ivopts.c.
> * tree-ssa-loop-ivopts.c (compute_max_addr_offset): Delete.
> (addr_list, addr_offset_valid_p): New.
> (split_address_groups): Check offset validity with above function.
> (gt-tree-ssa-loop-ivopts.h): Include.
>
>>>>
>>>>
>>>>
>>>>> Thanks,
>>>>> bin
>>>>> 2017-04-11 Bin Cheng <bin.cheng@arm.com>
>>>>>
>>>>> * tree-ssa-loop-ivopts.c (compute_max_addr_offset): Delete.
>>>>> (addr_offset_valid_p): New function.
>>>>> (split_address_groups): Check offset validity with above function.