On Wed, Jul 01, 2015 at 10:59:15AM +0100, Richard Sandiford wrote: > Trevor Saunders <tbsau...@tbsaunde.org> writes: > > On Tue, Jun 16, 2015 at 09:45:56AM +0100, Richard Sandiford wrote: > >> I couldn't tell whether the unusual name of the function > >> ("gt_cleare_cache") is deliberate or not, but I left it be. > > > > I'm not sure what's particularly unusual about it? > > "cleare" rather than "clear".
oh! I didn't notice that but yeah its wierd probably just being a bad typist. > >> + static int > >> + keep_cache_entry (tree_int_map *&m) > > > > I think we could now change the interface to take const T *? I imagine > > inlining may get rid of the extra indirection anyway, but it feels > > cleaner anyway. > > Yeah, good point. > > >> + - An optional static function named 'keep_cache_entry'. This > >> + function is provided only for garbage-collected elements that > >> + are not marked by the normal gc mark pass. It describes what > >> + what should happen to the element at the end of the gc mark phase. > >> + The return value should be: > >> + - 0 if the element should be deleted > >> + - 1 if the element should be kept and needs to be marked > >> + - -1 if the element should be kept and is already marked. > >> + Returning -1 rather than 1 is purely an optimization. > > > > In theory using an enum seems better, but I'm not sure if the extra > > verbosity makes it better in practice. > > Yeah, I wondered about an enum but it seemed like overkill. > > >> + static int > >> + keep_cache_entry (T &e) > >> { > >> - if (e != HTAB_EMPTY_ENTRY && e != HTAB_DELETED_ENTRY && > >> !ggc_marked_p (e)) > >> - e = static_cast<T> (HTAB_DELETED_ENTRY); > >> + return ggc_marked_p (e) ? -1 : 0; > > > > hmm, this is the only place where -1 is used right? I believe this > > case only works if the hash table is storing pointers to things in gc > > memory, and only keeps things that get marked by other paths. I > > believe that means -1 is only a very small optimization because in the > > case we return -1 all we save is the check that that pointer itself is > > marked. > > Right. But it's an optimisation that we had before and I think we > should keep it. The -1 case is in the generic traits rather than > "user" code. > > > So i'm tempted to change this interface to just return a bool. > > Of course it would be nice if the compiler could inline enough to > > actually optimize out the redundant check if the pointer is makred, but > > that might take some shuffling code around. > > In practice all calls will return from {0, 1} or {0, -1}, so assuming > sensible inlining, no caller will use all three paths. yeah Trev > > Thanks, > Richard >