Move df_live and df_md bitmaps to own obstacks

On Thu, Jun 10, 2010 at 3:41 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jun 10, 2010 at 4:35 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi,
>> this patch moves df_live and df_md bitmaps to own obstacks.  I believe those
>> are all important df problems that depends on bitmap performance?
>>
>> Bootstrapped/regtested x86_64-linux (with df checking)
>> OK?
>>
>> Honza
>>
>>        * df-problems.c (df_live_problem_data): Add live_bitmaps.
>>        (df_live_alloc): Initialize problem data and live_osbtacks.
>>        (df_live_finalize): Remove obstack, problem data; do not
>>        clear all bitmaps.
>>        (df_live_top_dump, df_live_bottom_dump): Do not dump old
>>        data when not allocated.
>>        (df_live_verify_solution_start): Do not allocate problem data.
>>        (df_live_verify_solution_end): Check if out is allocated.
>>        (struct df_md_problem_data): New structure.
>>        (df_md_alloc): Allocate problem data.
>>        (df_md_free): Free problem data; do not clear bitmaps.
>>
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44498
>

We have


   if (!df_live_scratch)
-    df_live_scratch = BITMAP_ALLOC (NULL);
+    df_live_scratch = BITMAP_ALLOC (&problem_data->live_bitmaps);
...
+      bitmap_obstack_release (&problem_data->live_bitmaps);

       if (df_live_scratch)
        BITMAP_FREE (df_live_scratch);

We are calling bitmap_obstack_free after bitmap_obstack_release is called. Does
this patch make any senses?

On Thu, Jun 10, 2010 at 04:23:07PM -0700, H.J. Lu wrote:
> We have
> 
> 
>    if (!df_live_scratch)
> -    df_live_scratch = BITMAP_ALLOC (NULL);
> +    df_live_scratch = BITMAP_ALLOC (&problem_data->live_bitmaps);
> ...
> +      bitmap_obstack_release (&problem_data->live_bitmaps);
> 
>        if (df_live_scratch)
>         BITMAP_FREE (df_live_scratch);
> 
> We are calling bitmap_obstack_free after bitmap_obstack_release is called. Does
> this patch make any senses?
> 
> 
> Index: df-problems.c
> ===================================================================
> --- df-problems.c	(revision 160549)
> +++ df-problems.c	(working copy)
> @@ -1645,8 +1645,8 @@ df_live_free (void)
>        free (df_live->block_info);
>        bitmap_obstack_release (&problem_data->live_bitmaps);
> 
> -      if (df_live_scratch)
> -	BITMAP_FREE (df_live_scratch);
> +      /* df_live_scratch is allocated from problem_data->live_bitmaps.  */
> +      df_live_scratch = (bitmap) NULL;

Wouldn't it be better to move that
if (df_live_scratch)
  BITMAP_FREE (df_live_scratch);
before bitmap_obstack_release?  BITMAP_FREE is pretty cheap operation
and if you don't do it, the statistics won't be updated.

	Jakub

>> Index: df-problems.c
>> ===================================================================
>> --- df-problems.c     (revision 160549)
>> +++ df-problems.c     (working copy)
>> @@ -1645,8 +1645,8 @@ df_live_free (void)
>>        free (df_live->block_info);
>>        bitmap_obstack_release (&problem_data->live_bitmaps);
>>
>> -      if (df_live_scratch)
>> -     BITMAP_FREE (df_live_scratch);
>> +      /* df_live_scratch is allocated from problem_data->live_bitmaps.  */
>> +      df_live_scratch = (bitmap) NULL;
>
> Wouldn't it be better to move that
> if (df_live_scratch)
>  BITMAP_FREE (df_live_scratch);
> before bitmap_obstack_release?  BITMAP_FREE is pretty cheap operation
> and if you don't do it, the statistics won't be updated.

Yes.  Both patches make sense, but Jakub's is better.

Paolo