On Mon, 2020-01-13 at 21:58 -0500, David Malcolm wrote:
> On Tue, 2020-01-14 at 00:55 +0100, Jakub Jelinek wrote:
> > On Mon, Jan 13, 2020 at 06:42:06PM -0500, David Malcolm wrote:
> > > Thanks. Does it have warnings, though?
> > >
> > > My attempt was similar, but ran into warnings from -Wclass-
> > > memaccess in
> > > four places, like this:
> > >
> > > ../../src/gcc/hash-map-traits.h:102:12: warning: ‘void*
> > > memset(void*,
> > > int, size_t)’ clearing an object of type ‘struct
> > > hash_map<tree_node*,
> > > std::pair<tree_node*, tree_node*> >::hash_entry’ with no trivial
> > > copy-
> > > assignment; use assignment or value-initialization instead [-
> > > Wclass-
> > > memaccess]
> > > 102 | memset (entry, 0, sizeof (T) * count);
> > > | ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > where the types in question are:
> > >
> > > (1)
> > > struct hash_map<tree_node*, std::pair<tree_node*, tree_node*>
> > > > ::hash_entry
> > > ../../src/gcc/tree-data-ref.c:844:17: required from here
> >
> > I don't understand how there could be new warnings.
> > The patch doesn't add any new memsets, all it does is if (0) code
> > in
> > alloc_entries for certain traits and in empty_slow stops using
> > memset
> > for some traits and uses mark_empty loop there instead.
>
> Your patch didn't add new memsets; mine did - clearly I misspoke when
> saying they were similar, mine had a somewhat different approach.
>
> Sorry for the noise, and thanks for your patch.
>
> I've extended your patch to cover the various hash traits in the
> analyzer and it passes the analyzer's tests. I also added in the
> selftests from my older patch.
>
> I examined the generated code, and it seems correct:
>
> * cfg.s correctly has a call to memset (even with no optimization)
> for
> hash_table<bb_copy_hasher>::empty_slow
>
> * the hash_map example with nonzero empty for the analyzer:
> analyzer/program-state.s: sm_state_map::remap_svalue_ids: hash_map
> <svalue_id, entry_t> map_t; correctly shows a loop of calls to
> mark_empty
>
> * a hash_map example with zero empty: tree-ssa.s edge_var_maps
> correctly has a memset in the empty_slow, even with no optimization.
>
> ...which is promising.
>
>
> For the graphite case, I wondered what happens if both is_empty and
> is_deleted are true; looking at hash-table.h, sometimes one is
> checked
> first, sometimes the other, but I don't think it matters for this
> case:; you have empty_zero_p = false,so it uses mark_empty, rather
> than
> trying to memset, which is correct - empty_zero_p being true can be
> thought of as a "it is safe to optimize this hash_table/hash_map by
> treating empty slots as all zero", if you will.
>
>
> I'm trying a bootstrap build and full regression suite now, for all
> languages (with graphite enabled, I believe). Will post the patches
> if
> it succeeds...
>
> > This was non-bootstrapped build, but I didn't see new warnings in
> > there,
> > and for tree-data-ref.c which you've mentioned I've tried to
> > compile
> > with installed trunk compiler and didn't get any warnings either.
> >
> > Jakub
>
> Thanks! Sorry again about getting this wrong.
>
> Dave
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu (in
conjuction with the analyzer patch kit, which it fixes)
OK for master?
gcc/cp/ChangeLog: Jakub Jelinek <ja...@redhat.com>
* cp-gimplify.c (source_location_table_entry_hash::empty_zero_p):
New static constant.
* cp-tree.h (named_decl_hash::empty_zero_p): Likewise.
(struct named_label_hash::empty_zero_p): Likewise.
* decl2.c (mangled_decl_hash::empty_zero_p): Likewise.
gcc/ChangeLog: Jakub Jelinek <ja...@redhat.com>,
David Malcolm <dmalc...@redhat.com>
* attribs.c (excl_hash_traits::empty_zero_p): New static constant.
* gcov.c (function_start_pair_hash::empty_zero_p): Likewise.
* graphite.c (struct sese_scev_hash::empty_zero_p): Likewise.
* hash-map-tests.c (selftest::test_nonzero_empty_key): New selftest.
(selftest::hash_map_tests_c_tests): Call it.
* hash-map-traits.h (simple_hashmap_traits::empty_zero_p):
New static constant, using the value of = H::empty_zero_p.
(unbounded_hashmap_traits::empty_zero_p): Likewise, using the value
from default_hash_traits <Value>.
* hash-map.h (hash_map::empty_zero_p): Likewise, using the value
from Traits.
* hash-set-tests.c (value_hash_traits::empty_zero_p): Likewise.
* hash-table.h (hash_table::alloc_entries): Guard the loop of
calls to mark_empty with !Descriptor::empty_zero_p.
(hash_table::empty_slow): Conditionalize the memset call with a
check that Descriptor::empty_zero_p; otherwise, loop through the
entries calling mark_empty on them.
* hash-traits.h (int_hash::empty_zero_p): New static constant.
(pointer_hash::empty_zero_p): Likewise.
(pair_hash::empty_zero_p): Likewise.
* ipa-devirt.c (default_hash_traits <type_pair>::empty_zero_p):
Likewise.
* ipa-prop.c (ipa_bit_ggc_hash_traits::empty_zero_p): Likewise.
(ipa_vr_ggc_hash_traits::empty_zero_p): Likewise.
* profile.c (location_triplet_hash::empty_zero_p): Likewise.
* sanopt.c (sanopt_tree_triplet_hash::empty_zero_p): Likewise.
(sanopt_tree_couple_hash::empty_zero_p): Likewise.
* tree-hasher.h (int_tree_hasher::empty_zero_p): Likewise.
* tree-ssa-sccvn.c (vn_ssa_aux_hasher::empty_zero_p): Likewise.
* tree-vect-slp.c (bst_traits::empty_zero_p): Likewise.
* tree-vectorizer.h
(default_hash_traits<scalar_cond_masked_key>::empty_zero_p):
Likewise.
---
gcc/attribs.c | 2 ++
gcc/cp/cp-gimplify.c | 2 ++
gcc/cp/cp-tree.h | 2 ++
gcc/cp/decl2.c | 1 +
gcc/gcov.c | 2 ++
gcc/graphite.c | 1 +
gcc/hash-map-tests.c | 22 ++++++++++++++++++++++
gcc/hash-map-traits.h | 2 ++
gcc/hash-map.h | 1 +
gcc/hash-set-tests.c | 2 ++
gcc/hash-table.h | 10 +++++++---
gcc/hash-traits.h | 3 +++
gcc/ipa-devirt.c | 1 +
gcc/ipa-prop.c | 2 ++
gcc/profile.c | 2 ++
gcc/sanopt.c | 4 ++++
gcc/tree-hasher.h | 1 +
gcc/tree-ssa-sccvn.c | 1 +
gcc/tree-vect-slp.c | 1 +
gcc/tree-vectorizer.h | 2 ++
20 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/gcc/attribs.c b/gcc/attribs.c
index ca89443eb3e..c66d4ae2c06 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -2048,6 +2048,8 @@ struct excl_hash_traits: typed_noop_remove<excl_pair>
x = value_type (NULL, NULL);
}
+ static const bool empty_zero_p = false;
+
static void mark_empty (value_type &x)
{
x = value_type ("", "");
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 827d240d11a..f401fe93fea 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -3043,6 +3043,8 @@ struct source_location_table_entry_hash
ref.var = NULL_TREE;
}
+ static const bool empty_zero_p = true;
+
static void
mark_empty (source_location_table_entry &ref)
{
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 98572bdbad1..c3ca4c8dace 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -900,6 +900,7 @@ struct named_decl_hash : ggc_remove <tree>
inline static hashval_t hash (const value_type decl);
inline static bool equal (const value_type existing, compare_type candidate);
+ static const bool empty_zero_p = true;
static inline void mark_empty (value_type &p) {p = NULL_TREE;}
static inline bool is_empty (value_type p) {return !p;}
@@ -1870,6 +1871,7 @@ struct named_label_hash : ggc_remove <named_label_entry *>
inline static hashval_t hash (value_type);
inline static bool equal (const value_type, compare_type);
+ static const bool empty_zero_p = true;
inline static void mark_empty (value_type &p) {p = NULL;}
inline static bool is_empty (value_type p) {return !p;}
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index a641667991f..042d6fa12df 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -120,6 +120,7 @@ struct mangled_decl_hash : ggc_remove <tree>
return candidate == name;
}
+ static const bool empty_zero_p = true;
static inline void mark_empty (value_type &p) {p = NULL_TREE;}
static inline bool is_empty (value_type p) {return !p;}
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 1dca3049777..a291bac3e9e 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -1225,6 +1225,8 @@ struct function_start_pair_hash : typed_noop_remove
<function_start>
ref.start_line = ~1U;
}
+ static const bool empty_zero_p = false;
+
static void
mark_empty (function_start &ref)
{
diff --git a/gcc/graphite.c b/gcc/graphite.c
index 0ac46766b15..27f1e486e1f 100644
--- a/gcc/graphite.c
+++ b/gcc/graphite.c
@@ -233,6 +233,7 @@ struct sese_scev_hash : typed_noop_remove <seir_cache_key>
&& operand_equal_p (key1.expr, key2.expr, 0));
}
static void mark_deleted (seir_cache_key &key) { key.expr = NULL_TREE; }
+ static const bool empty_zero_p = false;
static void mark_empty (seir_cache_key &key) { key.entry_dest = 0; }
static bool is_deleted (const seir_cache_key &key) { return !key.expr; }
static bool is_empty (const seir_cache_key &key) { return key.entry_dest ==
0; }
diff --git a/gcc/hash-map-tests.c b/gcc/hash-map-tests.c
index 743fb26d571..63574029065 100644
--- a/gcc/hash-map-tests.c
+++ b/gcc/hash-map-tests.c
@@ -280,6 +280,27 @@ test_map_of_type_with_ctor_and_dtor ()
}
}
+/* Test calling empty on a hash_map that has a key type with non-zero
+ "empty" value. */
+
+static void
+test_nonzero_empty_key ()
+{
+ typedef int_hash<int, INT_MIN, INT_MAX> IntHash;
+ hash_map<int, int, simple_hashmap_traits<IntHash, int> > x;
+
+ for (int i = 1; i != 32; ++i)
+ x.put (i, i);
+
+ ASSERT_EQ (x.get (0), NULL);
+ ASSERT_EQ (*x.get (1), 1);
+
+ x.empty ();
+
+ ASSERT_EQ (x.get (0), NULL);
+ ASSERT_EQ (x.get (1), NULL);
+}
+
/* Run all of the selftests within this file. */
void
@@ -288,6 +309,7 @@ hash_map_tests_c_tests ()
test_map_of_strings_to_int ();
test_map_of_int_to_strings ();
test_map_of_type_with_ctor_and_dtor ();
+ test_nonzero_empty_key ();
}
} // namespace selftest
diff --git a/gcc/hash-map-traits.h b/gcc/hash-map-traits.h
index 4764380b364..3b16be35f7d 100644
--- a/gcc/hash-map-traits.h
+++ b/gcc/hash-map-traits.h
@@ -36,6 +36,7 @@ struct simple_hashmap_traits
static inline hashval_t hash (const key_type &);
static inline bool equal_keys (const key_type &, const key_type &);
template <typename T> static inline void remove (T &);
+ static const bool empty_zero_p = H::empty_zero_p;
template <typename T> static inline bool is_empty (const T &);
template <typename T> static inline bool is_deleted (const T &);
template <typename T> static inline void mark_empty (T &);
@@ -113,6 +114,7 @@ template <typename Value>
struct unbounded_hashmap_traits
{
template <typename T> static inline void remove (T &);
+ static const bool empty_zero_p = default_hash_traits <Value>::empty_zero_p;
template <typename T> static inline bool is_empty (const T &);
template <typename T> static inline bool is_deleted (const T &);
template <typename T> static inline void mark_empty (T &);
diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index 7cb466767ea..5b8fd184e32 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -66,6 +66,7 @@ class GTY((user)) hash_map
return Traits::is_deleted (e);
}
+ static const bool empty_zero_p = Traits::empty_zero_p;
static void mark_empty (hash_entry &e) { Traits::mark_empty (e); }
static bool is_empty (const hash_entry &e) { return Traits::is_empty (e); }
diff --git a/gcc/hash-set-tests.c b/gcc/hash-set-tests.c
index 696e35e9be0..bb32094be20 100644
--- a/gcc/hash-set-tests.c
+++ b/gcc/hash-set-tests.c
@@ -199,6 +199,8 @@ struct value_hash_traits: int_hash<int, -1, -2>
base_type::mark_deleted (v.val);
}
+ static const bool empty_zero_p = false;
+
static void mark_empty (value_type &v)
{
base_type::mark_empty (v.val);
diff --git a/gcc/hash-table.h b/gcc/hash-table.h
index e0ddac5f578..a1423c78112 100644
--- a/gcc/hash-table.h
+++ b/gcc/hash-table.h
@@ -713,8 +713,9 @@ hash_table<Descriptor, Lazy,
nentries = ::ggc_cleared_vec_alloc<value_type> (n PASS_MEM_STAT);
gcc_assert (nentries != NULL);
- for (size_t i = 0; i < n; i++)
- mark_empty (nentries[i]);
+ if (!Descriptor::empty_zero_p)
+ for (size_t i = 0; i < n; i++)
+ mark_empty (nentries[i]);
return nentries;
}
@@ -867,8 +868,11 @@ hash_table<Descriptor, Lazy, Allocator>::empty_slow ()
m_size = nsize;
m_size_prime_index = nindex;
}
- else
+ else if (Descriptor::empty_zero_p)
memset ((void *) entries, 0, size * sizeof (value_type));
+ else
+ for (size_t i = 0; i < size; i++)
+ mark_empty (entries[i]);
m_n_deleted = 0;
m_n_elements = 0;
diff --git a/gcc/hash-traits.h b/gcc/hash-traits.h
index d259a41a418..3bca74c56ea 100644
--- a/gcc/hash-traits.h
+++ b/gcc/hash-traits.h
@@ -88,6 +88,7 @@ struct int_hash : typed_noop_remove <Type>
static inline hashval_t hash (value_type);
static inline bool equal (value_type existing, value_type candidate);
static inline void mark_deleted (Type &);
+ static const bool empty_zero_p = Empty == 0;
static inline void mark_empty (Type &);
static inline bool is_deleted (Type);
static inline bool is_empty (Type);
@@ -150,6 +151,7 @@ struct pointer_hash
static inline bool equal (const value_type &existing,
const compare_type &candidate);
static inline void mark_deleted (Type *&);
+ static const bool empty_zero_p = true;
static inline void mark_empty (Type *&);
static inline bool is_deleted (Type *);
static inline bool is_empty (Type *);
@@ -323,6 +325,7 @@ struct pair_hash
static inline bool equal (const value_type &, const compare_type &);
static inline void remove (value_type &);
static inline void mark_deleted (value_type &);
+ static const bool empty_zero_p = T1::empty_zero_p;
static inline void mark_empty (value_type &);
static inline bool is_deleted (const value_type &);
static inline bool is_empty (const value_type &);
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index b888186134c..f0031957375 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -150,6 +150,7 @@ struct default_hash_traits <type_pair>
{
return TYPE_UID (p.first) ^ TYPE_UID (p.second);
}
+ static const bool empty_zero_p = true;
static bool
is_empty (type_pair p)
{
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index ddb3c9b21bc..d90cdf24454 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -78,6 +78,7 @@ struct ipa_bit_ggc_hash_traits : public ggc_cache_remove
<ipa_bits *>
{
return a->value == b->value && a->mask == b->mask;
}
+ static const bool empty_zero_p = true;
static void
mark_empty (ipa_bits *&p)
{
@@ -123,6 +124,7 @@ struct ipa_vr_ggc_hash_traits : public ggc_cache_remove
<value_range *>
{
return a->equal_p (*b);
}
+ static const bool empty_zero_p = true;
static void
mark_empty (value_range *&p)
{
diff --git a/gcc/profile.c b/gcc/profile.c
index e124dc6173a..6a2de21c3bd 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -932,6 +932,8 @@ struct location_triplet_hash : typed_noop_remove
<location_triplet>
ref.lineno = -1;
}
+ static const bool empty_zero_p = false;
+
static void
mark_empty (location_triplet &ref)
{
diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 8a29abe99e2..619aae45a15 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -129,6 +129,8 @@ struct sanopt_tree_triplet_hash : typed_noop_remove
<sanopt_tree_triplet>
ref.t1 = reinterpret_cast<tree> (1);
}
+ static const bool empty_zero_p = true;
+
static void
mark_empty (sanopt_tree_triplet &ref)
{
@@ -184,6 +186,8 @@ struct sanopt_tree_couple_hash : typed_noop_remove
<sanopt_tree_couple>
ref.ptr = reinterpret_cast<tree> (1);
}
+ static const bool empty_zero_p = true;
+
static void
mark_empty (sanopt_tree_couple &ref)
{
diff --git a/gcc/tree-hasher.h b/gcc/tree-hasher.h
index 787d1ad6a62..9194d6227a2 100644
--- a/gcc/tree-hasher.h
+++ b/gcc/tree-hasher.h
@@ -40,6 +40,7 @@ struct int_tree_hasher
}
static void mark_deleted (value_type &v) { v.to = reinterpret_cast<tree>
(0x1); }
static bool is_empty (const value_type &v) { return v.to == NULL; }
+ static const bool empty_zero_p = true;
static void mark_empty (value_type &v) { v.to = NULL; }
static void remove (value_type &) {}
};
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 6f6b19eb165..3b27c50ef75 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -335,6 +335,7 @@ struct vn_ssa_aux_hasher : typed_noop_remove <vn_ssa_aux_t>
static inline hashval_t hash (const value_type &);
static inline bool equal (const value_type &, const compare_type &);
static inline void mark_deleted (value_type &) {}
+ static const bool empty_zero_p = true;
static inline void mark_empty (value_type &e) { e = NULL; }
static inline bool is_deleted (value_type &) { return false; }
static inline bool is_empty (value_type &e) { return e == NULL; }
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 9cb724b95ae..d164937b4b0 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -1193,6 +1193,7 @@ struct bst_traits
static inline bool equal (value_type existing, value_type candidate);
static inline bool is_empty (value_type x) { return !x.exists (); }
static inline bool is_deleted (value_type x) { return !x.exists (); }
+ static const bool empty_zero_p = true;
static inline void mark_empty (value_type &x) { x.release (); }
static inline void mark_deleted (value_type &x) { x.release (); }
static inline void remove (value_type &x) { x.release (); }
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 68cf96f6766..361f9a0741a 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -232,6 +232,8 @@ struct default_hash_traits<scalar_cond_masked_key>
&& operand_equal_p (existing.op1, candidate.op1, 0));
}
+ static const bool empty_zero_p = true;
+
static inline void
mark_empty (value_type &v)
{
--
2.21.0
