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
