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?

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.