On Mon, Mar 25, 2019 at 03:10:04PM -0400, Jason Merrill wrote:
> > 2) has the false -> true fixed
> > 3) ditto, but furthermore is moved out of add_capture into the lambda
> > introducer parsing routine only, tests for [this, this] and [this, *this]
> > etc. are done using LAMBDA_EXPR_THIS_CAPTURE and the duplicate check for
> > capture_id is simplified too.
>
> I needed to make the below change to avoid crashing with
> --enable-gather-detailed-mem-stats, make sense? Incidentally, why not set
CCing Martin on this, I really don't know how the mem-stats.h stuff is
supposed to work and how it works on reallocations.
Seems release_instance_overhead is used in expand method without the second
argument true and in the dtor with true, so the latter likely does both
what expand needs to do and undo what register_descriptor did.
And because register_descriptor has been called, it needs to be undone, but
as no register_instance_overhead has been called, that part should not be.
We don't have unregister_descriptor or something similar though.
So, the fix probably needs to add something in mem-stats.h and do what your
patch did + else if (m_gather_mem_stats) call this new mem_stat.h method
after the if (!Lazy || m_entries) block.
> m_size to 0 when we haven't allocated anything yet?
The reason for that was that the hash table ctor allows to specify the
expected size, but the later method where in the lazy allocation case
alloc_entries is actually called don't. So, if m_size and the
corresponding m_size_prime_index are set to 0 (or the latter perhaps to
INT_MAX or similar to crash badly), then we'd need to hardcode what size we
choose on the first allocation, instead of allowing the user to pick it up
during the construction.
BTW, why don't do something like following (dunno if it is valid not to
define the static data member out of the class unless it would be constexpr
or inline, but it isn't really ODR used, is it?), because at least looking
at currently emitted code without --enable-gather-detailed-mem-stats, we
emit this statistics gathering code everywhere except in the ctors where
optimizers can clearly see that m_gather_mem_stats is false. But we usually
don't see that in the destructor or in the methods, especially when they
aren't inlined. Martin?
--- gcc/hash-table.h 2019-03-21 22:59:16.501772695 +0100
+++ gcc/hash-table.h 2019-03-25 21:19:32.589325346 +0100
@@ -562,7 +562,11 @@ private:
bool m_ggc;
/* If we should gather memory statistics for the table. */
+#if GATHER_STATISTICS
bool m_gather_mem_stats;
+#else
+ static const bool m_gather_mem_stats = false;
+#endif
};
/* As mem-stats.h heavily utilizes hash maps (hash tables), we have to include
@@ -583,7 +587,10 @@ hash_table<Descriptor, Lazy, Allocator>:
mem_alloc_origin origin
MEM_STAT_DECL) :
m_n_elements (0), m_n_deleted (0), m_searches (0), m_collisions (0),
- m_ggc (ggc), m_gather_mem_stats (gather_mem_stats)
+ m_ggc (ggc)
+#if GATHER_STATISTICS
+ , m_gather_mem_stats (gather_mem_stats)
+#endif
{
unsigned int size_prime_index;
@@ -610,8 +617,10 @@ hash_table<Descriptor, Lazy, Allocator>:
mem_alloc_origin origin
MEM_STAT_DECL) :
m_n_elements (h.m_n_elements), m_n_deleted (h.m_n_deleted),
- m_searches (0), m_collisions (0), m_ggc (ggc),
- m_gather_mem_stats (gather_mem_stats)
+ m_searches (0), m_collisions (0), m_ggc (ggc)
+#if GATHER_STATISTICS
+ , m_gather_mem_stats (gather_mem_stats)
+#endif
{
size_t size = h.m_size;
Jakub
