Re: [PATCH 3/3] vec: use inexact growth where possible.
On Tue, Sep 1, 2020 at 1:26 PM Martin Liška wrote: > > On 8/27/20 10:54 AM, Richard Biener wrote: > > On Wed, Aug 26, 2020 at 11:02 PM Jeff Law wrote: > >> > >> On Tue, 2020-08-11 at 13:37 +0200, Martin Liška wrote: > >>> From cc1d41a469d76f2f8e4f44bed788ace77a1c6d62 Mon Sep 17 00:00:00 2001 > >>> From: Martin Liska > >>> Date: Mon, 10 Aug 2020 12:09:19 +0200 > >>> Subject: [PATCH 3/3] vec: use inexact growth where possible. > >>> > >>> gcc/ChangeLog: > >>> > >>>* cfgrtl.c (rtl_create_basic_block): Use default value for > >>>growth vector function. > >>>* gimple.c (gimple_set_bb): Likewise. > >>>* symbol-summary.h: Likewise. > >>>* tree-cfg.c (init_empty_tree_cfg_for_function): Likewise. > >>>(build_gimple_cfg): Likewise. > >>>(create_bb): Likewise. > >>>(move_block_to_fn): Likewise. > >> I'll note that in some cases we were avoiding exponential growth in our > >> new size > >> computations. Presumably the inexact growth support will do something > >> similar, > >> even if it's not exactly the same. Right? Assuming that's the case this > >> is OK > >> too. > > > > @@ -183,12 +183,12 @@ init_empty_tree_cfg_for_function (struct function *fn) > > last_basic_block_for_fn (fn) = NUM_FIXED_BLOCKS; > > vec_alloc (basic_block_info_for_fn (fn), initial_cfg_capacity); > > vec_safe_grow_cleared (basic_block_info_for_fn (fn), > > -initial_cfg_capacity, true); > > +initial_cfg_capacity); > > > > /* Build a mapping of labels to their associated blocks. */ > > vec_alloc (label_to_block_map_for_fn (fn), initial_cfg_capacity); > > vec_safe_grow_cleared (label_to_block_map_for_fn (fn), > > -initial_cfg_capacity, true); > > +initial_cfg_capacity); > > > > this is odd now at least since we explicitely alloc initial_cfg_capacity. > > IMHO we should instead use > > > > basic_block_info_for_fn (fn)->quick_grow_cleared (initial_cfg_capacity) > > > > here? > > Yes, with the following small change: > > + vec_safe_grow_cleared (basic_block_info_for_fn (fn), > +initial_cfg_capacity, true); > > as basic_block_info_for_fn (fn) is NULL. But we alloc it previously to exact size? OK, you can drop the vec_alloc calls of course. > Thanks for review, > Martin > > > > > The rest looks OK and along the plan. > > > > Thanks, > > Richard. > > > >> jeff > >> >
Re: [PATCH 3/3] vec: use inexact growth where possible.
On 8/27/20 10:54 AM, Richard Biener wrote: On Wed, Aug 26, 2020 at 11:02 PM Jeff Law wrote: On Tue, 2020-08-11 at 13:37 +0200, Martin Liška wrote: From cc1d41a469d76f2f8e4f44bed788ace77a1c6d62 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Mon, 10 Aug 2020 12:09:19 +0200 Subject: [PATCH 3/3] vec: use inexact growth where possible. gcc/ChangeLog: * cfgrtl.c (rtl_create_basic_block): Use default value for growth vector function. * gimple.c (gimple_set_bb): Likewise. * symbol-summary.h: Likewise. * tree-cfg.c (init_empty_tree_cfg_for_function): Likewise. (build_gimple_cfg): Likewise. (create_bb): Likewise. (move_block_to_fn): Likewise. I'll note that in some cases we were avoiding exponential growth in our new size computations. Presumably the inexact growth support will do something similar, even if it's not exactly the same. Right? Assuming that's the case this is OK too. @@ -183,12 +183,12 @@ init_empty_tree_cfg_for_function (struct function *fn) last_basic_block_for_fn (fn) = NUM_FIXED_BLOCKS; vec_alloc (basic_block_info_for_fn (fn), initial_cfg_capacity); vec_safe_grow_cleared (basic_block_info_for_fn (fn), -initial_cfg_capacity, true); +initial_cfg_capacity); /* Build a mapping of labels to their associated blocks. */ vec_alloc (label_to_block_map_for_fn (fn), initial_cfg_capacity); vec_safe_grow_cleared (label_to_block_map_for_fn (fn), -initial_cfg_capacity, true); +initial_cfg_capacity); this is odd now at least since we explicitely alloc initial_cfg_capacity. IMHO we should instead use basic_block_info_for_fn (fn)->quick_grow_cleared (initial_cfg_capacity) here? Yes, with the following small change: + vec_safe_grow_cleared (basic_block_info_for_fn (fn), +initial_cfg_capacity, true); as basic_block_info_for_fn (fn) is NULL. Thanks for review, Martin The rest looks OK and along the plan. Thanks, Richard. jeff
Re: [PATCH 3/3] vec: use inexact growth where possible.
On Wed, Aug 26, 2020 at 11:02 PM Jeff Law wrote: > > On Tue, 2020-08-11 at 13:37 +0200, Martin Liška wrote: > > From cc1d41a469d76f2f8e4f44bed788ace77a1c6d62 Mon Sep 17 00:00:00 2001 > > From: Martin Liska > > Date: Mon, 10 Aug 2020 12:09:19 +0200 > > Subject: [PATCH 3/3] vec: use inexact growth where possible. > > > > gcc/ChangeLog: > > > > * cfgrtl.c (rtl_create_basic_block): Use default value for > > growth vector function. > > * gimple.c (gimple_set_bb): Likewise. > > * symbol-summary.h: Likewise. > > * tree-cfg.c (init_empty_tree_cfg_for_function): Likewise. > > (build_gimple_cfg): Likewise. > > (create_bb): Likewise. > > (move_block_to_fn): Likewise. > I'll note that in some cases we were avoiding exponential growth in our new > size > computations. Presumably the inexact growth support will do something > similar, > even if it's not exactly the same. Right? Assuming that's the case this is > OK > too. @@ -183,12 +183,12 @@ init_empty_tree_cfg_for_function (struct function *fn) last_basic_block_for_fn (fn) = NUM_FIXED_BLOCKS; vec_alloc (basic_block_info_for_fn (fn), initial_cfg_capacity); vec_safe_grow_cleared (basic_block_info_for_fn (fn), -initial_cfg_capacity, true); +initial_cfg_capacity); /* Build a mapping of labels to their associated blocks. */ vec_alloc (label_to_block_map_for_fn (fn), initial_cfg_capacity); vec_safe_grow_cleared (label_to_block_map_for_fn (fn), -initial_cfg_capacity, true); +initial_cfg_capacity); this is odd now at least since we explicitely alloc initial_cfg_capacity. IMHO we should instead use basic_block_info_for_fn (fn)->quick_grow_cleared (initial_cfg_capacity) here? The rest looks OK and along the plan. Thanks, Richard. > jeff >
Re: [PATCH 3/3] vec: use inexact growth where possible.
On Tue, 2020-08-11 at 13:37 +0200, Martin Liška wrote: > From cc1d41a469d76f2f8e4f44bed788ace77a1c6d62 Mon Sep 17 00:00:00 2001 > From: Martin Liska > Date: Mon, 10 Aug 2020 12:09:19 +0200 > Subject: [PATCH 3/3] vec: use inexact growth where possible. > > gcc/ChangeLog: > > * cfgrtl.c (rtl_create_basic_block): Use default value for > growth vector function. > * gimple.c (gimple_set_bb): Likewise. > * symbol-summary.h: Likewise. > * tree-cfg.c (init_empty_tree_cfg_for_function): Likewise. > (build_gimple_cfg): Likewise. > (create_bb): Likewise. > (move_block_to_fn): Likewise. I'll note that in some cases we were avoiding exponential growth in our new size computations. Presumably the inexact growth support will do something similar, even if it's not exactly the same. Right? Assuming that's the case this is OK too. jeff
[PATCH 3/3] vec: use inexact growth where possible.
>From cc1d41a469d76f2f8e4f44bed788ace77a1c6d62 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Mon, 10 Aug 2020 12:09:19 +0200 Subject: [PATCH 3/3] vec: use inexact growth where possible. gcc/ChangeLog: * cfgrtl.c (rtl_create_basic_block): Use default value for growth vector function. * gimple.c (gimple_set_bb): Likewise. * symbol-summary.h: Likewise. * tree-cfg.c (init_empty_tree_cfg_for_function): Likewise. (build_gimple_cfg): Likewise. (create_bb): Likewise. (move_block_to_fn): Likewise. --- gcc/cfgrtl.c | 8 ++-- gcc/gimple.c | 7 +-- gcc/symbol-summary.h | 13 +++-- gcc/tree-cfg.c | 27 +-- 4 files changed, 15 insertions(+), 40 deletions(-) diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c index 03fa688fed6..0e65537f255 100644 --- a/gcc/cfgrtl.c +++ b/gcc/cfgrtl.c @@ -374,12 +374,8 @@ rtl_create_basic_block (void *headp, void *endp, basic_block after) /* Grow the basic block array if needed. */ if ((size_t) last_basic_block_for_fn (cfun) >= basic_block_info_for_fn (cfun)->length ()) -{ - size_t new_size = - (last_basic_block_for_fn (cfun) - + (last_basic_block_for_fn (cfun) + 3) / 4); - vec_safe_grow_cleared (basic_block_info_for_fn (cfun), new_size, true); -} +vec_safe_grow_cleared (basic_block_info_for_fn (cfun), + last_basic_block_for_fn (cfun) + 1); n_basic_blocks_for_fn (cfun)++; diff --git a/gcc/gimple.c b/gcc/gimple.c index 337a83a9154..a174ed48e0b 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -1689,12 +1689,7 @@ gimple_set_bb (gimple *stmt, basic_block bb) vec_safe_length (label_to_block_map_for_fn (cfun)); LABEL_DECL_UID (t) = uid = cfun->cfg->last_label_uid++; if (old_len <= (unsigned) uid) - { - unsigned new_len = 3 * uid / 2 + 1; - - vec_safe_grow_cleared (label_to_block_map_for_fn (cfun), - new_len, true); - } + vec_safe_grow_cleared (label_to_block_map_for_fn (cfun), uid + 1); } (*label_to_block_map_for_fn (cfun))[uid] = bb; diff --git a/gcc/symbol-summary.h b/gcc/symbol-summary.h index fa1df5c8015..a38eb1db778 100644 --- a/gcc/symbol-summary.h +++ b/gcc/symbol-summary.h @@ -354,11 +354,8 @@ public: id = this->m_symtab->assign_summary_id (node); if ((unsigned int)id >= m_vector->length ()) - { - int newlen = this->m_symtab->cgraph_max_summary_id; - vec_safe_reserve (m_vector, newlen - m_vector->length ()); - m_vector->quick_grow_cleared (newlen); - } + vec_safe_grow_cleared (m_vector, + this->m_symtab->cgraph_max_summary_id); if ((*m_vector)[id] == NULL) (*m_vector)[id] = this->allocate_new (); @@ -815,11 +812,7 @@ public: id = this->m_symtab->assign_summary_id (edge); if ((unsigned)id >= m_vector->length ()) - { - int newlen = this->m_symtab->edges_max_summary_id; - m_vector->reserve (newlen - m_vector->length ()); - m_vector->quick_grow_cleared (newlen); - } + vec_safe_grow_cleared (m_vector, this->m_symtab->edges_max_summary_id); if ((*m_vector)[id] == NULL) (*m_vector)[id] = this->allocate_new (); diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 2bae2eeddba..b79cf6c6d4c 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -183,12 +183,12 @@ init_empty_tree_cfg_for_function (struct function *fn) last_basic_block_for_fn (fn) = NUM_FIXED_BLOCKS; vec_alloc (basic_block_info_for_fn (fn), initial_cfg_capacity); vec_safe_grow_cleared (basic_block_info_for_fn (fn), - initial_cfg_capacity, true); + initial_cfg_capacity); /* Build a mapping of labels to their associated blocks. */ vec_alloc (label_to_block_map_for_fn (fn), initial_cfg_capacity); vec_safe_grow_cleared (label_to_block_map_for_fn (fn), - initial_cfg_capacity, true); + initial_cfg_capacity); SET_BASIC_BLOCK_FOR_FN (fn, ENTRY_BLOCK, ENTRY_BLOCK_PTR_FOR_FN (fn)); SET_BASIC_BLOCK_FOR_FN (fn, EXIT_BLOCK, EXIT_BLOCK_PTR_FOR_FN (fn)); @@ -232,7 +232,7 @@ build_gimple_cfg (gimple_seq seq) if (basic_block_info_for_fn (cfun)->length () < (size_t) n_basic_blocks_for_fn (cfun)) vec_safe_grow_cleared (basic_block_info_for_fn (cfun), - n_basic_blocks_for_fn (cfun), true); + n_basic_blocks_for_fn (cfun)); /* To speed up statement iterator walks, we first purge dead labels. */ cleanup_dead_labels (); @@ -681,12 +681,8 @@ create_bb (void *h, void *e, basic_block after) /* Grow the basic block array if needed. */ if ((size_t) last_basic_block_for_fn (cfun) == basic_block_info_for_fn (cfun)->length ()) -{ - size_t new_size = - (last_basic_block_for_fn (cfun) - + (last_basic_block_for_fn (cfun) + 3) / 4); - vec_safe_grow_cleared (basic_block_info_for_fn (cfun), new_size, true); -} +vec_safe_grow_cleared (basic_block_info_for_fn (cfun), + last_basic_block_for_fn (c