On Tue, 2020-01-14 at 08:55 +0100, Richard Biener wrote: > On Mon, 13 Jan 2020, David Malcolm wrote: > > > I posted the initial version of the analyzer patch kit on 2019-11- > > 15, > > shortly before the close of stage 1. > > > > Jeff reviewed (most of) the latest version of the kit on Friday, > > and > > said: > > > > > I'm not going to have time to finish #22 or #37 -- hell, I'm not > > > even > > > supposed to be working today :-) > > > > > > I'd suggest committing now and we can iterate on any issues. The > > > code > > > is appropriately conditionalized, so it shouldn't affect anyone > > > that > > > doesn't ask for it and it was submitted prior to stage1 close. > > > > > > I would also suggest that we have a fairly loose policy for these > > > bits > > > right now. Again, they're conditionally compiled and it's > > > effectively > > > "tech preview", so if we muck something up, the after-effects are > > > relatively small. > > > > Unfortunately, I didn't resolve the hash_table/hash_map issue > > referred to here: > > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00734.html > > where r279139 on 2019-12-09 introduced the assumption that empty > > hash_table entries and empty hash_map keys are all zeroes. > > > > The analyzer uses hash_map::empty in a single place with a hash_map > > where the "empty" key value is all-ones. > > > > Unfortunately, without a fix for this issue, the analyzer can hang, > > leading to DejaGnu hanging, which I clearly don't want to push to > > master as-is. > > > > The patch here fixes the issue: > > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00776.html > > but Jakub has expressed concern about the effect on code generated > > for the compiler. > > > > As noted here: > > https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00651.html > > gcc successfully converts this back to a memset to 0 for hash_table > > at -O2, but not for hash_map, since it can't "know" that it's OK to > > clobber the values as well as the keys; it instead generates a loop > > that zeroes the keys without touching the values. > > > > I've been experimenting with adding template specializations to try > > to allow for memset to 0 for hash_map, but haven't been successful > > (e.g. std::is_pod is C++11-only); my closest attempt so far is at: > > > > https://dmalcolm.fedorapeople.org/gcc/2020-01-13/0001-FIXME-experiments-with-fixing-hash_map-empty.patch > > which at least expresses the memset to 0 without needing > > optimization for hash_table of *pointer* types, but I wasn't able > > to > > do the same for hash_map, today at least. > > > > If the hash_map::empty patch is unacceptable, I've also > > successfully > > B&R-ed the following kludge to the analyzer, which avoids using > > hash_map::empty at the single place where it's problematic. This > > kludge is entirely confined to the analyzer. > > > > I'd like to get the analyzer code into gcc 10, but I appreciate > > it's > > now stage 4. Jakub suggested on IRC that with approval before the > > end of stage 3 (which it was), there may be some flexibility if we > > get it in soon and I take steps to minimize disruption. > > > > Some options: > > (a) the patch to fix hash_table::empty, and the analyzer kit > > (b) the analyzer kit with the following kludge > > (c) someone with better C++-fu than me figure out a way to get the > > memset optimization for hash_map with 0-valued empty (or to give me > > some suggestions) > > (d) not merge the analyzer for gcc 10 > > > > My preferred approach is option (a), but option (b) is confined to > > the analyzer. > > > > Thoughts? > > (b) > > we can iterate on (a)/(c) if deemed necessary but also this can be > done > during next stage1.
Thanks. Jakub's proposed fix worked, so I've effectively gone with (c). I've rebased and squashed the analyzer patch kit and squashed patch 2 of the hash_table fix into it, and re-tested it successfully, so I've pushed it to master (as 757bf1dff5e8cee34c0a75d06140ca972bfecfa7). I'm going to work through the various followup patches I had on my branch and re-test and push to master those that seem appropriate. Dave