Re: [PATCH 5/9] ENABLE_CHECKING refactoring: pool allocators

[Richard Biener <richard.guenther@gmail.com>]

On Mon, Oct 19, 2015 at 2:09 AM, Mikhail Maltsev <maltsevm@gmail.com> wrote:
> On 10/06/2015 03:45 PM, Richard Biener wrote:
>> On Tue, Oct 6, 2015 at 2:41 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
>>> On 10/06/2015 01:32 AM, Mikhail Maltsev wrote:
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2015-10-05  Mikhail Maltsev  <maltsevm@gmail.com>
>>>>
>>>>         * alloc-pool.h (base_pool_allocator::initialize, ::allocate,
>>>>         ::remove): Adjust to use CHECKING_P.
>>>
>>>
>>> Why CHECKING_P for these and not flag_checking as elsewhere?
>>
>> Probably because they are in inline functions and thus possibly would
>> affect optimization.  Not sure why they are inline functions in the
>> first place...  I'd agree to using flag_checking here.
>>
>> Richard.
>>
>>>
>>> Bernd
>
> Adjusted. Note: I had to include 'options.h' into 'alloc-pool.h' in order to use
> flag_checking.

Ugh (stupid templates).

@@ -387,10 +389,10 @@ base_pool_allocator <TBlockAllocator>::allocate ()
       block = m_virgin_free_list;
       header = (allocation_pool_list*) allocation_object::get_data (block);
       header->next = NULL;
-#ifdef ENABLE_CHECKING
+
       /* Mark the element to be free.  */
-      ((allocation_object*) block)->id = 0;
-#endif
+      if (flag_checking)
+       ((allocation_object*) block)->id = 0;

just set id to zero unconditionally.  That'll be faster than checking
flag_checking.

-#ifdef ENABLE_CHECKING
   /* Set the ID for element.  */
-  allocation_object::get_instance (header)->id = m_id;
-#endif
+  if (flag_checking)
+    allocation_object::get_instance (header)->id = m_id;

likewise.

Given that, the pool initialization itself can do with unconditonally
computing the id as well, thus

-#ifdef ENABLE_CHECKING
-  /* Increase the last used ID and use it for this pool.
-     ID == 0 is used for free elements of pool so skip it.  */
-  last_id++;
-  if (last_id == 0)
-    last_id++;
+  if (flag_checking)
+    {
+      /* Increase the last used ID and use it for this pool.
+        ID == 0 is used for free elements of pool so skip it.  */
+      last_id++;
+      if (last_id == 0)
+       last_id++;

-  m_id = last_id;
-#endif
+      m_id = last_id;
+    }

make that unconditionally enabled as well.

-#ifdef ENABLE_CHECKING
-  gcc_assert (object
+  gcc_checking_assert (object
              /* Check if we free more than we allocated, which is Bad (TM).  */
              && m_elts_free < m_elts_allocated
              /* Check whether the PTR was allocated from POOL.  */
              && m_id == allocation_object::get_instance (object)->id);

-  memset (object, 0xaf, size);
-
-  /* Mark the element to be free.  */
-  allocation_object::get_instance (object)->id = 0;
-#endif
+  if (flag_checking)
+    {
+      memset (object, 0xaf, size);
+      /* Mark the element to be free.  */
+      allocation_object::get_instance (object)->id = 0;
+    }


guard the assert with flag_checking and do the id set unconditionally
(for consistency).

Ok with those changes.

Thanks,
Richard.


> --
> Regards,
>     Mikhail Maltsev
>
> 2015-10-19  Mikhail Maltsev  <maltsevm@gmail.com>
>
>         * alloc-pool.h (base_pool_allocator ::initialize, ::allocate,
>         ::remove): Adjust to use flag_checking.