Re: [PATCH] Add sem_item::m_hash_set (PR ipa/78309) (v2)
> On 11/21/2016 04:50 PM, Jan Hubicka wrote:
> > OK,
> > thanks!
> > Honza
>
> Hi.
>
> Patch to trunk is already installed. Equal patch can be installed to gcc-6
> branch,
> however gcc-5 branch needs more hunks to be adjusted. I did so, both patches
> survive
> regression tests and the patch for gcc-5 provides equal results for gimp w/
> -flto and -O2.
>
> Ready to be installed to both branches?
OK,
thanks!
Honza
> Thanks,
> Martin
> >From 4a1dfd4ef43e9bf5fe7c9252a5d9e48cc1d9d444 Mon Sep 17 00:00:00 2001
> From: marxin
> Date: Tue, 22 Nov 2016 11:06:37 +0100
> Subject: [PATCH] [PATCH] Add sem_item::m_hash_set (PR ipa/78309)
>
> gcc/ChangeLog:
>
> 2016-11-16 Martin Liska
>
> PR ipa/78309
> * ipa-icf.c (void sem_item::set_hash): Update m_hash_set.
> (sem_function::get_hash): Use the new field.
> (sem_function::parse): Remove an argument from ctor.
> (sem_variable::parse): Likewise.
> (sem_variable::get_hash): Use the new field.
> (sem_item_optimizer::read_section): Use new ctor and set hash.
> * ipa-icf.h: _hash is removed from sem_item::sem_item,
> sem_variable::sem_variable, sem_function::sem_function.
> ---
> gcc/ipa-icf.c | 73
> ---
> gcc/ipa-icf.h | 26 -
> 2 files changed, 50 insertions(+), 49 deletions(-)
>
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index 729bc06..216e4ed 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -172,16 +172,13 @@ symbol_compare_collection::symbol_compare_collection
> (symtab_node *node)
>
> /* Constructor for key value pair, where _ITEM is key and _INDEX is a
> target. */
>
> -sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index):
> - item (_item), index (_index)
> +sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index)
> +: item (_item), index (_index)
> {
> }
>
> -/* Semantic item constructor for a node of _TYPE, where STACK is used
> - for bitmap memory allocation. */
> -
> -sem_item::sem_item (sem_item_type _type,
> - bitmap_obstack *stack): type(_type), hash(0)
> +sem_item::sem_item (sem_item_type _type, bitmap_obstack *stack)
> +: type(_type), hash(-1), m_hash_set (false)
> {
>setup (stack);
> }
> @@ -191,8 +188,8 @@ sem_item::sem_item (sem_item_type _type,
> with computed _HASH. */
>
> sem_item::sem_item (sem_item_type _type, symtab_node *_node,
> - hashval_t _hash, bitmap_obstack *stack): type(_type),
> - node (_node), hash (_hash)
> + bitmap_obstack *stack)
> +: type(_type), node (_node), hash (-1), m_hash_set (false)
> {
>decl = node->decl;
>setup (stack);
> @@ -268,6 +265,12 @@ sem_item::target_supports_symbol_aliases_p (void)
> #endif
> }
>
> +void sem_item::set_hash (hashval_t h)
> +{
> + hash = h;
> + m_hash_set = true;
> +}
> +
> /* Semantic function constructor that uses STACK as bitmap memory stack. */
>
> sem_function::sem_function (bitmap_obstack *stack): sem_item (FUNC, stack),
> @@ -277,12 +280,8 @@ sem_function::sem_function (bitmap_obstack *stack):
> sem_item (FUNC, stack),
>bb_sorted.create (0);
> }
>
> -/* Constructor based on callgraph node _NODE with computed hash _HASH.
> -Bitmap STACK is used for memory allocation. */
> -sem_function::sem_function (cgraph_node *node, hashval_t hash,
> - bitmap_obstack *stack):
> - sem_item (FUNC, node, hash, stack),
> - m_checker (NULL), m_compared_func (NULL)
> +sem_function::sem_function (cgraph_node *node, bitmap_obstack *stack)
> +: sem_item (FUNC, node, stack), m_checker (NULL), m_compared_func (NULL)
> {
>bb_sizes.create (0);
>bb_sorted.create (0);
> @@ -315,7 +314,7 @@ sem_function::get_bb_hash (const sem_bb *basic_block)
> hashval_t
> sem_function::get_hash (void)
> {
> - if(!hash)
> + if(!m_hash_set)
> {
>inchash::hash hstate;
>hstate.add_int (177454); /* Random number for function type. */
> @@ -345,7 +344,7 @@ sem_function::get_hash (void)
>hstate.add_flag (DECL_CXX_CONSTRUCTOR_P (decl));
>hstate.add_flag (DECL_CXX_DESTRUCTOR_P (decl));
>
> - hash = hstate.end ();
> + set_hash (hstate.end ());
> }
>
>return hash;
> @@ -1533,7 +1532,7 @@ sem_function::parse (cgraph_node *node, bitmap_obstack
> *stack)
>|| DECL_STATIC_DESTRUCTOR (node->decl))
> return NULL;
>
> - sem_function *f = new sem_function (node, 0, stack);
> + sem_function *f = new sem_function (node, stack);
>
>f->init ();
>
> @@ -1637,19 +1636,12 @@ sem_function::bb_dict_test (vec *bb_dict, int
> source, int target)
> return (*bb_dict)[source] == target;
> }
>
> -
> -/* Semantic variable constructor that uses STACK as bitmap memory stack. */
> -
> sem_variable::sem_variable (bitmap_obstack *stack): sem_item (VAR, stack)
> {
> }
>
> -/* Constructor based on varpool node _NODE with computed hash _HASH.
> -
Re: [PATCH] Add sem_item::m_hash_set (PR ipa/78309) (v2)
On 11/21/2016 04:50 PM, Jan Hubicka wrote:
> OK,
> thanks!
> Honza
Hi.
Patch to trunk is already installed. Equal patch can be installed to gcc-6
branch,
however gcc-5 branch needs more hunks to be adjusted. I did so, both patches
survive
regression tests and the patch for gcc-5 provides equal results for gimp w/
-flto and -O2.
Ready to be installed to both branches?
Thanks,
Martin
>From 4a1dfd4ef43e9bf5fe7c9252a5d9e48cc1d9d444 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 22 Nov 2016 11:06:37 +0100
Subject: [PATCH] [PATCH] Add sem_item::m_hash_set (PR ipa/78309)
gcc/ChangeLog:
2016-11-16 Martin Liska
PR ipa/78309
* ipa-icf.c (void sem_item::set_hash): Update m_hash_set.
(sem_function::get_hash): Use the new field.
(sem_function::parse): Remove an argument from ctor.
(sem_variable::parse): Likewise.
(sem_variable::get_hash): Use the new field.
(sem_item_optimizer::read_section): Use new ctor and set hash.
* ipa-icf.h: _hash is removed from sem_item::sem_item,
sem_variable::sem_variable, sem_function::sem_function.
---
gcc/ipa-icf.c | 73 ---
gcc/ipa-icf.h | 26 -
2 files changed, 50 insertions(+), 49 deletions(-)
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 729bc06..216e4ed 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -172,16 +172,13 @@ symbol_compare_collection::symbol_compare_collection (symtab_node *node)
/* Constructor for key value pair, where _ITEM is key and _INDEX is a target. */
-sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index):
- item (_item), index (_index)
+sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index)
+: item (_item), index (_index)
{
}
-/* Semantic item constructor for a node of _TYPE, where STACK is used
- for bitmap memory allocation. */
-
-sem_item::sem_item (sem_item_type _type,
- bitmap_obstack *stack): type(_type), hash(0)
+sem_item::sem_item (sem_item_type _type, bitmap_obstack *stack)
+: type(_type), hash(-1), m_hash_set (false)
{
setup (stack);
}
@@ -191,8 +188,8 @@ sem_item::sem_item (sem_item_type _type,
with computed _HASH. */
sem_item::sem_item (sem_item_type _type, symtab_node *_node,
- hashval_t _hash, bitmap_obstack *stack): type(_type),
- node (_node), hash (_hash)
+ bitmap_obstack *stack)
+: type(_type), node (_node), hash (-1), m_hash_set (false)
{
decl = node->decl;
setup (stack);
@@ -268,6 +265,12 @@ sem_item::target_supports_symbol_aliases_p (void)
#endif
}
+void sem_item::set_hash (hashval_t h)
+{
+ hash = h;
+ m_hash_set = true;
+}
+
/* Semantic function constructor that uses STACK as bitmap memory stack. */
sem_function::sem_function (bitmap_obstack *stack): sem_item (FUNC, stack),
@@ -277,12 +280,8 @@ sem_function::sem_function (bitmap_obstack *stack): sem_item (FUNC, stack),
bb_sorted.create (0);
}
-/* Constructor based on callgraph node _NODE with computed hash _HASH.
-Bitmap STACK is used for memory allocation. */
-sem_function::sem_function (cgraph_node *node, hashval_t hash,
- bitmap_obstack *stack):
- sem_item (FUNC, node, hash, stack),
- m_checker (NULL), m_compared_func (NULL)
+sem_function::sem_function (cgraph_node *node, bitmap_obstack *stack)
+: sem_item (FUNC, node, stack), m_checker (NULL), m_compared_func (NULL)
{
bb_sizes.create (0);
bb_sorted.create (0);
@@ -315,7 +314,7 @@ sem_function::get_bb_hash (const sem_bb *basic_block)
hashval_t
sem_function::get_hash (void)
{
- if(!hash)
+ if(!m_hash_set)
{
inchash::hash hstate;
hstate.add_int (177454); /* Random number for function type. */
@@ -345,7 +344,7 @@ sem_function::get_hash (void)
hstate.add_flag (DECL_CXX_CONSTRUCTOR_P (decl));
hstate.add_flag (DECL_CXX_DESTRUCTOR_P (decl));
- hash = hstate.end ();
+ set_hash (hstate.end ());
}
return hash;
@@ -1533,7 +1532,7 @@ sem_function::parse (cgraph_node *node, bitmap_obstack *stack)
|| DECL_STATIC_DESTRUCTOR (node->decl))
return NULL;
- sem_function *f = new sem_function (node, 0, stack);
+ sem_function *f = new sem_function (node, stack);
f->init ();
@@ -1637,19 +1636,12 @@ sem_function::bb_dict_test (vec *bb_dict, int source, int target)
return (*bb_dict)[source] == target;
}
-
-/* Semantic variable constructor that uses STACK as bitmap memory stack. */
-
sem_variable::sem_variable (bitmap_obstack *stack): sem_item (VAR, stack)
{
}
-/* Constructor based on varpool node _NODE with computed hash _HASH.
-Bitmap STACK is used for memory allocation. */
-
-sem_variable::sem_variable (varpool_node *node, hashval_t _hash,
- bitmap_obstack *stack): sem_item(VAR,
- node, _hash, stack)
+sem_variable::sem_variable (varpool_node *node, bitmap_obstack *stack)
+: sem_item (VAR, node, stack)
{
gcc_checking_assert (node);
gcc_checking_assert (get_node ());
@@ -1947,7 +1939,7 @@ sem_variable::parse (varpool_node
Re: [PATCH] Add sem_item::m_hash_set (PR ipa/78309) (v2)
> On 11/15/2016 05:46 PM, Jan Hubicka wrote:
> > Yep, zero is definitly valid hash value:0
> >
> > Patch is OK. We may consider backporting it to release branches.
> > Honza
>
> Thanks, sending v2 as I found an error in the previous version.
> Changes from last version:
> - comments for ctors are just in header file, not duplicated in ipa-icf.c
> - hash argument has been removed from ctors
> - ctors GNU coding style has been fixed and unified
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?
> Martin
> >From 5be0ca49cfa67ca848002d6fe008ef4c2885bd40 Mon Sep 17 00:00:00 2001
> From: marxin
> Date: Fri, 11 Nov 2016 16:15:20 +0100
> Subject: [PATCH] Add sem_item::m_hash_set (PR ipa/78309)
>
> gcc/ChangeLog:
>
> 2016-11-16 Martin Liska
>
> PR ipa/78309
> * ipa-icf.c (void sem_item::set_hash): Update m_hash_set.
> (sem_function::get_hash): Use the new field.
> (sem_function::parse): Remove an argument from ctor.
> (sem_variable::parse): Likewise.
> (sem_variable::get_hash): Use the new field.
> (sem_item_optimizer::read_section): Use new ctor and set hash.
> * ipa-icf.h: _hash is removed from sem_item::sem_item,
> sem_variable::sem_variable, sem_function::sem_function.
OK,
thanks!
Honza
> ---
> gcc/ipa-icf.c | 64
> ---
> gcc/ipa-icf.h | 17
> 2 files changed, 35 insertions(+), 46 deletions(-)
>
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index 1ab67f3..212e406 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -131,27 +131,20 @@ symbol_compare_collection::symbol_compare_collection
> (symtab_node *node)
>
> /* Constructor for key value pair, where _ITEM is key and _INDEX is a
> target. */
>
> -sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index):
> - item (_item), index (_index)
> +sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index)
> +: item (_item), index (_index)
> {
> }
>
> -/* Semantic item constructor for a node of _TYPE, where STACK is used
> - for bitmap memory allocation. */
> -
> -sem_item::sem_item (sem_item_type _type,
> - bitmap_obstack *stack): type (_type), m_hash (0)
> +sem_item::sem_item (sem_item_type _type, bitmap_obstack *stack)
> +: type (_type), m_hash (-1), m_hash_set (false)
> {
>setup (stack);
> }
>
> -/* Semantic item constructor for a node of _TYPE, where STACK is used
> - for bitmap memory allocation. The item is based on symtab node _NODE
> - with computed _HASH. */
> -
> sem_item::sem_item (sem_item_type _type, symtab_node *_node,
> - hashval_t _hash, bitmap_obstack *stack): type(_type),
> - node (_node), m_hash (_hash)
> + bitmap_obstack *stack)
> +: type (_type), node (_node), m_hash (-1), m_hash_set (false)
> {
>decl = node->decl;
>setup (stack);
> @@ -230,23 +223,20 @@ sem_item::target_supports_symbol_aliases_p (void)
> void sem_item::set_hash (hashval_t hash)
> {
>m_hash = hash;
> + m_hash_set = true;
> }
>
> /* Semantic function constructor that uses STACK as bitmap memory stack. */
>
> -sem_function::sem_function (bitmap_obstack *stack): sem_item (FUNC, stack),
> - m_checker (NULL), m_compared_func (NULL)
> +sem_function::sem_function (bitmap_obstack *stack)
> +: sem_item (FUNC, stack), m_checker (NULL), m_compared_func (NULL)
> {
>bb_sizes.create (0);
>bb_sorted.create (0);
> }
>
> -/* Constructor based on callgraph node _NODE with computed hash _HASH.
> -Bitmap STACK is used for memory allocation. */
> -sem_function::sem_function (cgraph_node *node, hashval_t hash,
> - bitmap_obstack *stack):
> - sem_item (FUNC, node, hash, stack),
> - m_checker (NULL), m_compared_func (NULL)
> +sem_function::sem_function (cgraph_node *node, bitmap_obstack *stack)
> +: sem_item (FUNC, node, stack), m_checker (NULL), m_compared_func (NULL)
> {
>bb_sizes.create (0);
>bb_sorted.create (0);
> @@ -279,7 +269,7 @@ sem_function::get_bb_hash (const sem_bb *basic_block)
> hashval_t
> sem_function::get_hash (void)
> {
> - if (!m_hash)
> + if (!m_hash_set)
> {
>inchash::hash hstate;
>hstate.add_int (177454); /* Random number for function type. */
> @@ -1704,7 +1694,7 @@ sem_function::parse (cgraph_node *node, bitmap_obstack
> *stack)
>|| DECL_STATIC_DESTRUCTOR (node->decl))
> return NULL;
>
> - sem_function *f = new sem_function (node, 0, stack);
> + sem_function *f = new sem_function (node, stack);
>
>f->init ();
>
> @@ -1807,19 +1797,12 @@ sem_function::bb_dict_test (vec *bb_dict, int
> source, int target)
> return (*bb_dict)[source] == target;
> }
>
> -
> -/* Semantic variable constructor that uses STACK as bitmap memory stack. */
> -
> sem_variable::sem_variable (bitmap_obstack *stack): sem_item (VAR, stack)
> {
> }
>
> -/* Constructor base
Re: [PATCH] Add sem_item::m_hash_set (PR ipa/78309) (v2)
On 11/15/2016 05:46 PM, Jan Hubicka wrote:
> Yep, zero is definitly valid hash value:0
>
> Patch is OK. We may consider backporting it to release branches.
> Honza
Thanks, sending v2 as I found an error in the previous version.
Changes from last version:
- comments for ctors are just in header file, not duplicated in ipa-icf.c
- hash argument has been removed from ctors
- ctors GNU coding style has been fixed and unified
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
>From 5be0ca49cfa67ca848002d6fe008ef4c2885bd40 Mon Sep 17 00:00:00 2001
From: marxin
Date: Fri, 11 Nov 2016 16:15:20 +0100
Subject: [PATCH] Add sem_item::m_hash_set (PR ipa/78309)
gcc/ChangeLog:
2016-11-16 Martin Liska
PR ipa/78309
* ipa-icf.c (void sem_item::set_hash): Update m_hash_set.
(sem_function::get_hash): Use the new field.
(sem_function::parse): Remove an argument from ctor.
(sem_variable::parse): Likewise.
(sem_variable::get_hash): Use the new field.
(sem_item_optimizer::read_section): Use new ctor and set hash.
* ipa-icf.h: _hash is removed from sem_item::sem_item,
sem_variable::sem_variable, sem_function::sem_function.
---
gcc/ipa-icf.c | 64 ---
gcc/ipa-icf.h | 17
2 files changed, 35 insertions(+), 46 deletions(-)
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 1ab67f3..212e406 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -131,27 +131,20 @@ symbol_compare_collection::symbol_compare_collection (symtab_node *node)
/* Constructor for key value pair, where _ITEM is key and _INDEX is a target. */
-sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index):
- item (_item), index (_index)
+sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index)
+: item (_item), index (_index)
{
}
-/* Semantic item constructor for a node of _TYPE, where STACK is used
- for bitmap memory allocation. */
-
-sem_item::sem_item (sem_item_type _type,
- bitmap_obstack *stack): type (_type), m_hash (0)
+sem_item::sem_item (sem_item_type _type, bitmap_obstack *stack)
+: type (_type), m_hash (-1), m_hash_set (false)
{
setup (stack);
}
-/* Semantic item constructor for a node of _TYPE, where STACK is used
- for bitmap memory allocation. The item is based on symtab node _NODE
- with computed _HASH. */
-
sem_item::sem_item (sem_item_type _type, symtab_node *_node,
- hashval_t _hash, bitmap_obstack *stack): type(_type),
- node (_node), m_hash (_hash)
+ bitmap_obstack *stack)
+: type (_type), node (_node), m_hash (-1), m_hash_set (false)
{
decl = node->decl;
setup (stack);
@@ -230,23 +223,20 @@ sem_item::target_supports_symbol_aliases_p (void)
void sem_item::set_hash (hashval_t hash)
{
m_hash = hash;
+ m_hash_set = true;
}
/* Semantic function constructor that uses STACK as bitmap memory stack. */
-sem_function::sem_function (bitmap_obstack *stack): sem_item (FUNC, stack),
- m_checker (NULL), m_compared_func (NULL)
+sem_function::sem_function (bitmap_obstack *stack)
+: sem_item (FUNC, stack), m_checker (NULL), m_compared_func (NULL)
{
bb_sizes.create (0);
bb_sorted.create (0);
}
-/* Constructor based on callgraph node _NODE with computed hash _HASH.
-Bitmap STACK is used for memory allocation. */
-sem_function::sem_function (cgraph_node *node, hashval_t hash,
- bitmap_obstack *stack):
- sem_item (FUNC, node, hash, stack),
- m_checker (NULL), m_compared_func (NULL)
+sem_function::sem_function (cgraph_node *node, bitmap_obstack *stack)
+: sem_item (FUNC, node, stack), m_checker (NULL), m_compared_func (NULL)
{
bb_sizes.create (0);
bb_sorted.create (0);
@@ -279,7 +269,7 @@ sem_function::get_bb_hash (const sem_bb *basic_block)
hashval_t
sem_function::get_hash (void)
{
- if (!m_hash)
+ if (!m_hash_set)
{
inchash::hash hstate;
hstate.add_int (177454); /* Random number for function type. */
@@ -1704,7 +1694,7 @@ sem_function::parse (cgraph_node *node, bitmap_obstack *stack)
|| DECL_STATIC_DESTRUCTOR (node->decl))
return NULL;
- sem_function *f = new sem_function (node, 0, stack);
+ sem_function *f = new sem_function (node, stack);
f->init ();
@@ -1807,19 +1797,12 @@ sem_function::bb_dict_test (vec *bb_dict, int source, int target)
return (*bb_dict)[source] == target;
}
-
-/* Semantic variable constructor that uses STACK as bitmap memory stack. */
-
sem_variable::sem_variable (bitmap_obstack *stack): sem_item (VAR, stack)
{
}
-/* Constructor based on varpool node _NODE with computed hash _HASH.
-Bitmap STACK is used for memory allocation. */
-
-sem_variable::sem_variable (varpool_node *node, hashval_t _hash,
- bitmap_obstack *stack): sem_item(VAR,
- node, _hash, stack)
+sem_variable::sem_variable (varpool_node *node, bitmap_obstack *stack)
+: sem_item (VAR, node, stack)
{
gcc_checking_assert (node);
gcc_checking_asser
