diff mbox

Add sem_item::m_hash_set (PR ipa/78309)

Message ID d936d1cf-07b2-b474-5cd3-d5d1c477885a@suse.cz
State New
Headers show

Commit Message

Martin Liška Nov. 15, 2016, 4:43 p.m. UTC
Hi.

As seen on ppc64le during compilation of Firefox with LTO, combining inchash value
with a pointer, enum value and an integer, one can eventually get zero value.
Thus I decided to introduce a new flag that would distinguish between not set hash value
and a valid and (possibly) zero value.

I've been running regression tests, ready to install after it finishes?
Martin

Comments

Jeff Law Nov. 15, 2016, 4:45 p.m. UTC | #1
On 11/15/2016 09:43 AM, Martin Liška wrote:
> Hi.

>

> As seen on ppc64le during compilation of Firefox with LTO, combining inchash value

> with a pointer, enum value and an integer, one can eventually get zero value.

> Thus I decided to introduce a new flag that would distinguish between not set hash value

> and a valid and (possibly) zero value.

>

> I've been running regression tests, ready to install after it finishes?

> Martin

>

>

> 0001-Add-sem_item-m_hash_set-PR-ipa-78309.patch

>

>

> From 952ca6f6c0f99bcd965825898970453fb413964e Mon Sep 17 00:00:00 2001

> From: marxin <mliska@suse.cz>

> Date: Fri, 11 Nov 2016 16:15:20 +0100

> Subject: [PATCH] Add sem_item::m_hash_set (PR ipa/78309)

>

> gcc/ChangeLog:

>

> 2016-11-15  Martin Liska  <mliska@suse.cz>

>

> 	PR ipa/78309

> 	* ipa-icf.c (void sem_item::set_hash): Update m_hash_set.

> 	(sem_function::get_hash): Make condition based on m_hash_set.

> 	(sem_variable::get_hash): Likewise.

> 	* ipa-icf.h (sem_item::m_hash_set): New property.

OK.

jeff
Jan Hubicka Nov. 15, 2016, 4:46 p.m. UTC | #2
> Hi.

> 

> As seen on ppc64le during compilation of Firefox with LTO, combining inchash value

> with a pointer, enum value and an integer, one can eventually get zero value.

> Thus I decided to introduce a new flag that would distinguish between not set hash value

> and a valid and (possibly) zero value.

> 

> I've been running regression tests, ready to install after it finishes?

> Martin


> >From 952ca6f6c0f99bcd965825898970453fb413964e Mon Sep 17 00:00:00 2001

> From: marxin <mliska@suse.cz>

> Date: Fri, 11 Nov 2016 16:15:20 +0100

> Subject: [PATCH] Add sem_item::m_hash_set (PR ipa/78309)

> 

> gcc/ChangeLog:

> 

> 2016-11-15  Martin Liska  <mliska@suse.cz>

> 

> 	PR ipa/78309

> 	* ipa-icf.c (void sem_item::set_hash): Update m_hash_set.

> 	(sem_function::get_hash): Make condition based on m_hash_set.

> 	(sem_variable::get_hash): Likewise.

> 	* ipa-icf.h (sem_item::m_hash_set): New property.

Yep, zero is definitly valid hash value:0

Patch is OK. We may consider backporting it to release branches.
Honza
diff mbox

Patch

From 952ca6f6c0f99bcd965825898970453fb413964e Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 11 Nov 2016 16:15:20 +0100
Subject: [PATCH] Add sem_item::m_hash_set (PR ipa/78309)

gcc/ChangeLog:

2016-11-15  Martin Liska  <mliska@suse.cz>

	PR ipa/78309
	* ipa-icf.c (void sem_item::set_hash): Update m_hash_set.
	(sem_function::get_hash): Make condition based on m_hash_set.
	(sem_variable::get_hash): Likewise.
	* ipa-icf.h (sem_item::m_hash_set): New property.
---
 gcc/ipa-icf.c | 10 ++++++----
 gcc/ipa-icf.h |  3 +++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 1ab67f3..4352fd0 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -140,7 +140,8 @@  sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index):
    for bitmap memory allocation.  */
 
 sem_item::sem_item (sem_item_type _type,
-		    bitmap_obstack *stack): type (_type), m_hash (0)
+		    bitmap_obstack *stack): type (_type), m_hash (0),
+		    m_hash_set (false)
 {
   setup (stack);
 }
@@ -151,7 +152,7 @@  sem_item::sem_item (sem_item_type _type,
 
 sem_item::sem_item (sem_item_type _type, symtab_node *_node,
 		    hashval_t _hash, bitmap_obstack *stack): type(_type),
-  node (_node), m_hash (_hash)
+  node (_node), m_hash (_hash), m_hash_set (true)
 {
   decl = node->decl;
   setup (stack);
@@ -230,6 +231,7 @@  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.  */
@@ -279,7 +281,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.  */
@@ -2116,7 +2118,7 @@  sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
 hashval_t
 sem_variable::get_hash (void)
 {
-  if (m_hash)
+  if (m_hash_set)
     return m_hash;
 
   /* All WPA streamed in symbols should have their hashes computed at compile
diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h
index d8de655..8dc3d31 100644
--- a/gcc/ipa-icf.h
+++ b/gcc/ipa-icf.h
@@ -274,6 +274,9 @@  protected:
   /* Hash of item.  */
   hashval_t m_hash;
 
+  /* Indicated whether a hash value has been set or not.  */
+  bool m_hash_set;
+
 private:
   /* Initialize internal data structures. Bitmap STACK is used for
      bitmap memory allocation process.  */
-- 
2.10.1