Mikhail Maltsev <malts...@gmail.com> writes:
> Hi, all.
> I'm pinging this patch:
> https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00030.html
A week and no reply, so FWIW: I really hope this goes in. malloc always
shows up high in my profiles too.
Just a couple of very minor things to show that I read the patch :-)
> diff --git a/gcc/virtual-memory.cc b/gcc/virtual-memory.cc
> new file mode 100644
> index 0000000..69bda37
> --- /dev/null
> +++ b/gcc/virtual-memory.cc
> @@ -0,0 +1,66 @@
> +/*
> + Copyright (C) 2015 Free Software Foundation, Inc.
Missing a description line. Same for the .h.
> +/* Free all memory allocated by memory_block_pool. */
Strictly speaking I think this is "all unused memory".
> + size_t size = reinterpret_cast<_obstack_chunk *> (chunk)->limit -
> + reinterpret_cast<char *>(chunk);
The emacs formatting rule would make this:
size_t size = (reinterpret_cast<_obstack_chunk *> (chunk)->limit
- reinterpret_cast<char *> (chunk));
Still not sure what the rule is supposed to be wrt space before "<"
or after ">".
> +#ifndef VIRTUAL_MEMORY_H
> +#define VIRTUAL_MEMORY_H
Tab indentation for the #define but not the #ifndef. (Think it should
be a space for both.)
> +/* Allocate single block. Reuse previously returned block, if possible. */
Pedantic, but: "a single block", "a previously returned block".
> +inline void *
> +memory_block_pool::allocate ()
> +{
> + if (instance.m_blocks == NULL)
> + return XNEWVEC (char, block_size);
> +
> + void *result = instance.m_blocks;
> + instance.m_blocks = instance.m_blocks->m_next;
> + return result;
> +}
> +
> +/* Return UNCAST_BLOCK to pool. */
"to the pool"
> +inline void
> +memory_block_pool::remove (void *uncast_block)
> +{
> + block_list *block = reinterpret_cast<block_list *> (uncast_block);
For aliasing purposes, should this instead be a placement new,
to show that a new block_list object is being created?
> +extern void *mempool_obstack_chunk_alloc(size_t) ATTRIBUTE_MALLOC;
> +extern void mempool_obstack_chunk_free(void *);
Space before "(".
Thanks,
Richard
