* [PATCH GCC8][07/33]Offset validity check in address expression
@ 2017-04-18 10:41 Bin Cheng
2017-04-24 10:37 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Bin Cheng @ 2017-04-18 10:41 UTC (permalink / raw)
To: gcc-patches; +Cc: nd
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
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?
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.
[-- Attachment #2: 0007-offset-check-in-group-split-20170220.txt --]
[-- Type: text/plain, Size: 5465 bytes --]
From fe33bd3fe9a1dbf1a10ae50bfeecc5a9d0b6c759 Mon Sep 17 00:00:00 2001
From: Bin Cheng <binche01@e108451-lin.cambridge.arm.com>
Date: Tue, 28 Feb 2017 16:19:27 +0000
Subject: [PATCH 07/33] offset-check-in-group-split-20170220.txt
---
gcc/tree-ssa-loop-ivopts.c | 103 +++++++++++++++++----------------------------
1 file changed, 38 insertions(+), 65 deletions(-)
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 840bde4..5ab1d29 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -2460,67 +2460,42 @@ find_interesting_uses_outside (struct ivopts_data *data, edge exit)
}
}
-/* Compute maximum offset of [base + offset] addressing mode
- for memory reference represented by USE. */
+/* Return TRUE if OFFSET is within the range of [base + offset] addressing
+ mode for memory reference represented by USE. */
-static HOST_WIDE_INT
-compute_max_addr_offset (struct iv_use *use)
+static bool
+addr_offset_valid_p (struct iv_use *use, HOST_WIDE_INT offset)
{
- int width;
rtx reg, addr;
- HOST_WIDE_INT i, off;
unsigned list_index, num;
addr_space_t as;
machine_mode mem_mode, addr_mode;
- 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)
{
- max_offset_list.safe_grow (list_index + MAX_MACHINE_MODE);
- for (; num < max_offset_list.length (); num++)
- max_offset_list[num] = -1;
+ addr_list.safe_grow_cleared (list_index + MAX_MACHINE_MODE);
+ for (; num < addr_list.length (); num++)
+ addr_list[num] = NULL;
}
- off = max_offset_list[list_index];
- if (off != -1)
- return off;
-
- addr_mode = targetm.addr_space.address_mode (as);
- reg = gen_raw_REG (addr_mode, LAST_VIRTUAL_REGISTER + 1);
- addr = gen_rtx_fmt_ee (PLUS, addr_mode, reg, NULL_RTX);
-
- width = GET_MODE_BITSIZE (addr_mode) - 1;
- if (width > (HOST_BITS_PER_WIDE_INT - 1))
- width = HOST_BITS_PER_WIDE_INT - 1;
-
- for (i = width; i > 0; i--)
+ addr = addr_list[list_index];
+ if (!addr)
{
- off = (HOST_WIDE_INT_1U << i) - 1;
- XEXP (addr, 1) = gen_int_mode (off, addr_mode);
- if (memory_address_addr_space_p (mem_mode, addr, as))
- break;
-
- /* For some strict-alignment targets, the offset must be naturally
- aligned. Try an aligned offset if mem_mode is not QImode. */
- off = (HOST_WIDE_INT_1U << i);
- if (off > GET_MODE_SIZE (mem_mode) && mem_mode != QImode)
- {
- off -= GET_MODE_SIZE (mem_mode);
- XEXP (addr, 1) = gen_int_mode (off, addr_mode);
- if (memory_address_addr_space_p (mem_mode, addr, as))
- break;
- }
+ addr_mode = targetm.addr_space.address_mode (as);
+ reg = gen_raw_REG (addr_mode, LAST_VIRTUAL_REGISTER + 1);
+ addr = gen_rtx_fmt_ee (PLUS, addr_mode, reg, NULL_RTX);
+ addr_list[list_index] = addr;
}
- if (i == 0)
- off = 0;
+ else
+ addr_mode = GET_MODE (addr);
- max_offset_list[list_index] = off;
- return off;
+ XEXP (addr, 1) = gen_int_mode (offset, addr_mode);
+ return (memory_address_addr_space_p (mem_mode, addr, as));
}
/* Comparison function to sort group in ascending order of addr_offset. */
@@ -2599,14 +2574,12 @@ static void
split_address_groups (struct ivopts_data *data)
{
unsigned int i, j;
- HOST_WIDE_INT max_offset = -1;
-
- /* Reset max offset to split all small groups. */
- if (split_small_address_groups_p (data))
- max_offset = 0;
+ /* Always split group. */
+ bool split_p = split_small_address_groups_p (data);
for (i = 0; i < data->vgroups.length (); i++)
{
+ struct iv_group *new_group = NULL;
struct iv_group *group = data->vgroups[i];
struct iv_use *use = group->vuses[0];
@@ -2615,29 +2588,29 @@ split_address_groups (struct ivopts_data *data)
if (group->vuses.length () == 1)
continue;
- if (max_offset != 0)
- max_offset = compute_max_addr_offset (use);
+ gcc_assert (group->type == USE_ADDRESS);
- for (j = 1; j < group->vuses.length (); j++)
+ for (j = 1; j < group->vuses.length ();)
{
struct iv_use *next = group->vuses[j];
+ HOST_WIDE_INT offset = next->addr_offset - use->addr_offset;
- /* Only uses with offset that can fit in offset part against
- the first use can be grouped together. */
- if (next->addr_offset - use->addr_offset
- > (unsigned HOST_WIDE_INT) max_offset)
- break;
+ /* Split group if aksed to, or the offset against the first
+ use can't fit in offset part of addressing mode. IV uses
+ having the same offset are still kept in one group. */
+ if (offset != 0 &&
+ (split_p || !addr_offset_valid_p (use, offset)))
+ {
+ if (!new_group)
+ new_group = record_group (data, group->type);
+ group->vuses.ordered_remove (j);
+ new_group->vuses.safe_push (next);
+ continue;
+ }
next->id = j;
next->group_id = group->id;
- }
- /* Split group. */
- if (j < group->vuses.length ())
- {
- struct iv_group *new_group = record_group (data, group->type);
- new_group->vuses.safe_splice (group->vuses);
- new_group->vuses.block_remove (0, j);
- group->vuses.truncate (j);
+ j++;
}
}
}
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH GCC8][07/33]Offset validity check in address expression
2017-04-18 10:41 [PATCH GCC8][07/33]Offset validity check in address expression Bin Cheng
@ 2017-04-24 10:37 ` Richard Biener
2017-05-02 17:08 ` Bin.Cheng
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2017-04-24 10:37 UTC (permalink / raw)
To: Bin Cheng; +Cc: gcc-patches, nd
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.
+ /* Split group if aksed to, or the offset against the first
+ use can't fit in offset part of addressing mode. IV uses
+ having the same offset are still kept in one group. */
+ if (offset != 0 &&
+ (split_p || !addr_offset_valid_p (use, offset)))
&& goes to the next line.
Richard.
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH GCC8][07/33]Offset validity check in address expression
2017-04-24 10:37 ` Richard Biener
@ 2017-05-02 17:08 ` Bin.Cheng
2017-05-03 9:54 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Bin.Cheng @ 2017-05-02 17:08 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
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.
Thanks,
bin
>
> + /* Split group if aksed to, or the offset against the first
> + use can't fit in offset part of addressing mode. IV uses
> + having the same offset are still kept in one group. */
> + if (offset != 0 &&
> + (split_p || !addr_offset_valid_p (use, offset)))
>
> && goes to the next line.
>
> Richard.
>
>
>
>> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH GCC8][07/33]Offset validity check in address expression
2017-05-02 17:08 ` Bin.Cheng
@ 2017-05-03 9:54 ` Richard Biener
2017-05-04 15:19 ` Bin.Cheng
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2017-05-03 9:54 UTC (permalink / raw)
To: Bin.Cheng; +Cc: gcc-patches
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.
Richard.
> Thanks,
> bin
>>
>> + /* Split group if aksed to, or the offset against the first
>> + use can't fit in offset part of addressing mode. IV uses
>> + having the same offset are still kept in one group. */
>> + if (offset != 0 &&
>> + (split_p || !addr_offset_valid_p (use, offset)))
>>
>> && goes to the next line.
>>
>> Richard.
>>
>>
>>
>>> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH GCC8][07/33]Offset validity check in address expression
2017-05-03 9:54 ` Richard Biener
@ 2017-05-04 15:19 ` Bin.Cheng
2017-05-05 7:23 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Bin.Cheng @ 2017-05-04 15:19 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]
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.
[-- Attachment #2: 0007-offset-check-in-group-split-20170501.txt --]
[-- Type: text/plain, Size: 5904 bytes --]
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 2411671..97259ac 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2484,7 +2484,7 @@ GTFILES = $(CPP_ID_DATA_H) $(srcdir)/input.h $(srcdir)/coretypes.h \
$(srcdir)/gimple-ssa.h \
$(srcdir)/tree-chkp.c \
$(srcdir)/tree-ssanames.c $(srcdir)/tree-eh.c $(srcdir)/tree-ssa-address.c \
- $(srcdir)/tree-cfg.c \
+ $(srcdir)/tree-cfg.c $(srcdir)/tree-ssa-loop-ivopts.c \
$(srcdir)/tree-dfa.c \
$(srcdir)/tree-iterator.c $(srcdir)/gimple-expr.c \
$(srcdir)/tree-chrec.h \
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 7caa40d..203b272 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -2460,67 +2460,36 @@ find_interesting_uses_outside (struct ivopts_data *data, edge exit)
}
}
-/* Compute maximum offset of [base + offset] addressing mode
- for memory reference represented by USE. */
+/* Return TRUE if OFFSET is within the range of [base + offset] addressing
+ mode for memory reference represented by USE. */
-static HOST_WIDE_INT
-compute_max_addr_offset (struct iv_use *use)
+static GTY (()) vec<rtx, va_gc> *addr_list;
+
+static bool
+addr_offset_valid_p (struct iv_use *use, HOST_WIDE_INT offset)
{
- int width;
rtx reg, addr;
- HOST_WIDE_INT i, off;
- unsigned list_index, num;
- addr_space_t as;
- machine_mode mem_mode, addr_mode;
- static vec<HOST_WIDE_INT> max_offset_list;
-
- as = TYPE_ADDR_SPACE (TREE_TYPE (use->iv->base));
- mem_mode = TYPE_MODE (TREE_TYPE (*use->op_p));
+ unsigned list_index;
+ addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (use->iv->base));
+ machine_mode addr_mode, mem_mode = TYPE_MODE (TREE_TYPE (*use->op_p));
- num = max_offset_list.length ();
list_index = (unsigned) as * MAX_MACHINE_MODE + (unsigned) mem_mode;
- if (list_index >= num)
- {
- max_offset_list.safe_grow (list_index + MAX_MACHINE_MODE);
- for (; num < max_offset_list.length (); num++)
- max_offset_list[num] = -1;
- }
+ if (list_index >= vec_safe_length (addr_list))
+ vec_safe_grow_cleared (addr_list, list_index + MAX_MACHINE_MODE);
- off = max_offset_list[list_index];
- if (off != -1)
- return off;
-
- addr_mode = targetm.addr_space.address_mode (as);
- reg = gen_raw_REG (addr_mode, LAST_VIRTUAL_REGISTER + 1);
- addr = gen_rtx_fmt_ee (PLUS, addr_mode, reg, NULL_RTX);
-
- width = GET_MODE_BITSIZE (addr_mode) - 1;
- if (width > (HOST_BITS_PER_WIDE_INT - 1))
- width = HOST_BITS_PER_WIDE_INT - 1;
-
- for (i = width; i > 0; i--)
+ addr = (*addr_list)[list_index];
+ if (!addr)
{
- off = (HOST_WIDE_INT_1U << i) - 1;
- XEXP (addr, 1) = gen_int_mode (off, addr_mode);
- if (memory_address_addr_space_p (mem_mode, addr, as))
- break;
-
- /* For some strict-alignment targets, the offset must be naturally
- aligned. Try an aligned offset if mem_mode is not QImode. */
- off = (HOST_WIDE_INT_1U << i);
- if (off > GET_MODE_SIZE (mem_mode) && mem_mode != QImode)
- {
- off -= GET_MODE_SIZE (mem_mode);
- XEXP (addr, 1) = gen_int_mode (off, addr_mode);
- if (memory_address_addr_space_p (mem_mode, addr, as))
- break;
- }
+ addr_mode = targetm.addr_space.address_mode (as);
+ reg = gen_raw_REG (addr_mode, LAST_VIRTUAL_REGISTER + 1);
+ addr = gen_rtx_fmt_ee (PLUS, addr_mode, reg, NULL_RTX);
+ (*addr_list)[list_index] = addr;
}
- if (i == 0)
- off = 0;
+ else
+ addr_mode = GET_MODE (addr);
- max_offset_list[list_index] = off;
- return off;
+ XEXP (addr, 1) = gen_int_mode (offset, addr_mode);
+ return (memory_address_addr_space_p (mem_mode, addr, as));
}
/* Comparison function to sort group in ascending order of addr_offset. */
@@ -2599,14 +2568,12 @@ static void
split_address_groups (struct ivopts_data *data)
{
unsigned int i, j;
- HOST_WIDE_INT max_offset = -1;
-
- /* Reset max offset to split all small groups. */
- if (split_small_address_groups_p (data))
- max_offset = 0;
+ /* Always split group. */
+ bool split_p = split_small_address_groups_p (data);
for (i = 0; i < data->vgroups.length (); i++)
{
+ struct iv_group *new_group = NULL;
struct iv_group *group = data->vgroups[i];
struct iv_use *use = group->vuses[0];
@@ -2615,29 +2582,29 @@ split_address_groups (struct ivopts_data *data)
if (group->vuses.length () == 1)
continue;
- if (max_offset != 0)
- max_offset = compute_max_addr_offset (use);
+ gcc_assert (group->type == USE_ADDRESS);
- for (j = 1; j < group->vuses.length (); j++)
+ for (j = 1; j < group->vuses.length ();)
{
struct iv_use *next = group->vuses[j];
+ HOST_WIDE_INT offset = next->addr_offset - use->addr_offset;
- /* Only uses with offset that can fit in offset part against
- the first use can be grouped together. */
- if (next->addr_offset - use->addr_offset
- > (unsigned HOST_WIDE_INT) max_offset)
- break;
+ /* Split group if aksed to, or the offset against the first
+ use can't fit in offset part of addressing mode. IV uses
+ having the same offset are still kept in one group. */
+ if (offset != 0 &&
+ (split_p || !addr_offset_valid_p (use, offset)))
+ {
+ if (!new_group)
+ new_group = record_group (data, group->type);
+ group->vuses.ordered_remove (j);
+ new_group->vuses.safe_push (next);
+ continue;
+ }
next->id = j;
next->group_id = group->id;
- }
- /* Split group. */
- if (j < group->vuses.length ())
- {
- struct iv_group *new_group = record_group (data, group->type);
- new_group->vuses.safe_splice (group->vuses);
- new_group->vuses.block_remove (0, j);
- group->vuses.truncate (j);
+ j++;
}
}
}
@@ -7854,3 +7821,5 @@ tree_ssa_iv_optimize (void)
tree_ssa_iv_optimize_finalize (&data);
}
+
+#include "gt-tree-ssa-loop-ivopts.h"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH GCC8][07/33]Offset validity check in address expression
2017-05-04 15:19 ` Bin.Cheng
@ 2017-05-05 7:23 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2017-05-05 7:23 UTC (permalink / raw)
To: Bin.Cheng; +Cc: gcc-patches
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-05 7:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 10:41 [PATCH GCC8][07/33]Offset validity check in address expression Bin Cheng
2017-04-24 10:37 ` Richard Biener
2017-05-02 17:08 ` Bin.Cheng
2017-05-03 9:54 ` Richard Biener
2017-05-04 15:19 ` Bin.Cheng
2017-05-05 7:23 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).