Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Aldy Hernandez]

On Mon, Apr 8, 2024 at 11:50 AM Richard Biener <rguenther@suse.de> wrote:
>
> The following fixes ranger bitmap allocation when invoked from IPA
> context where the global bitmap obstack possibly isn't initialized.
> Instead of trying to use one of the ranger obstacks the following
> simply initializes the global bitmap obstack around an active ranger.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu with bitmap_alloc
> instrumentation (all ICEs gone, so ranger was the only offender).
>
> OK?
>
> Thanks,
> Richard.
>
>         PR middle-end/114604
>         * gimple-range.cc (enable_ranger): Initialize the global
>         bitmap obstack.
>         (disable_ranger): Release it.
> ---
>  gcc/gimple-range.cc | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> index c16b776c1e3..4d3b1ce8588 100644
> --- a/gcc/gimple-range.cc
> +++ b/gcc/gimple-range.cc
> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
>  {
>    gimple_ranger *r;
>
> +  bitmap_obstack_initialize (NULL);
> +
>    gcc_checking_assert (!fun->x_range_query);
>    r = new gimple_ranger (use_imm_uses);
>    fun->x_range_query = r;
> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
>    gcc_checking_assert (fun->x_range_query);
>    delete fun->x_range_query;
>    fun->x_range_query = NULL;
> +
> +  bitmap_obstack_release (NULL);

Are you not allowed to initialize/use obstacks unless
bitmap_obstack_initialize(NULL) is called?  If so, wouldn't it be
better to lazily initialize it downstream (bitmap_alloc, or whomever
needs it initialized)?  Unless I'm misunderstanding something, it
doesn't seem like ranger is the correct place to fix this.

Aldy

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Jakub Jelinek]

On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
> >         PR middle-end/114604
> >         * gimple-range.cc (enable_ranger): Initialize the global
> >         bitmap obstack.
> >         (disable_ranger): Release it.
> > ---
> >  gcc/gimple-range.cc | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> > index c16b776c1e3..4d3b1ce8588 100644
> > --- a/gcc/gimple-range.cc
> > +++ b/gcc/gimple-range.cc
> > @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
> >  {
> >    gimple_ranger *r;
> >
> > +  bitmap_obstack_initialize (NULL);
> > +
> >    gcc_checking_assert (!fun->x_range_query);
> >    r = new gimple_ranger (use_imm_uses);
> >    fun->x_range_query = r;
> > @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
> >    gcc_checking_assert (fun->x_range_query);
> >    delete fun->x_range_query;
> >    fun->x_range_query = NULL;
> > +
> > +  bitmap_obstack_release (NULL);
> 
> Are you not allowed to initialize/use obstacks unless
> bitmap_obstack_initialize(NULL) is called?

You can use it with some other obstack, just not the default one.

> If so, wouldn't it be
> better to lazily initialize it downstream (bitmap_alloc, or whomever
> needs it initialized)?

No, you still need to decide where is the safe point to release it.
Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
the default one can nest (has associated nesting counter).  So, the above
patch just says that ranger starts using the default obstack in
enable_ranger and stops using it in disable_ranger and anything ranger
associated in the obstack can be freed at that point.
If one uses this from a pass which has its own
bitmap_obstack_{initializer,release} (NULL) around this, then the above will
not do anything, just bump the nesting counter and decrease it later.
But if some pass doesn't, i.e. the pass itself doesn't use the default
obstack, only ranger does, then the above seems like the right change to me.

> Unless I'm misunderstanding something, it
> doesn't seem like ranger is the correct place to fix this.

	Jakub

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Aldy Hernandez]

On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
> > >         PR middle-end/114604
> > >         * gimple-range.cc (enable_ranger): Initialize the global
> > >         bitmap obstack.
> > >         (disable_ranger): Release it.
> > > ---
> > >  gcc/gimple-range.cc | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> > > index c16b776c1e3..4d3b1ce8588 100644
> > > --- a/gcc/gimple-range.cc
> > > +++ b/gcc/gimple-range.cc
> > > @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
> > >  {
> > >    gimple_ranger *r;
> > >
> > > +  bitmap_obstack_initialize (NULL);
> > > +
> > >    gcc_checking_assert (!fun->x_range_query);
> > >    r = new gimple_ranger (use_imm_uses);
> > >    fun->x_range_query = r;
> > > @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
> > >    gcc_checking_assert (fun->x_range_query);
> > >    delete fun->x_range_query;
> > >    fun->x_range_query = NULL;
> > > +
> > > +  bitmap_obstack_release (NULL);
> >
> > Are you not allowed to initialize/use obstacks unless
> > bitmap_obstack_initialize(NULL) is called?
>
> You can use it with some other obstack, just not the default one.
>
> > If so, wouldn't it be
> > better to lazily initialize it downstream (bitmap_alloc, or whomever
> > needs it initialized)?
>
> No, you still need to decide where is the safe point to release it.
> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
> the default one can nest (has associated nesting counter).  So, the above
> patch just says that ranger starts using the default obstack in
> enable_ranger and stops using it in disable_ranger and anything ranger
> associated in the obstack can be freed at that point.

I thought ranger never used the default one:

$ grep bitmap_obstack_initialize *value* *range*
value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);

or even:

$ grep obstack.*NULL *value* *range*
value-range-storage.cc:    obstack_free (&m_obstack, NULL);
value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
value-range-storage.cc:    obstack_free (&m_obstack, NULL);

I'm obviously missing something here.

Aldy

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Richard Biener]

> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
> 
> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
>> 
>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
>>>>        PR middle-end/114604
>>>>        * gimple-range.cc (enable_ranger): Initialize the global
>>>>        bitmap obstack.
>>>>        (disable_ranger): Release it.
>>>> ---
>>>> gcc/gimple-range.cc | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>> 
>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
>>>> index c16b776c1e3..4d3b1ce8588 100644
>>>> --- a/gcc/gimple-range.cc
>>>> +++ b/gcc/gimple-range.cc
>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
>>>> {
>>>>   gimple_ranger *r;
>>>> 
>>>> +  bitmap_obstack_initialize (NULL);
>>>> +
>>>>   gcc_checking_assert (!fun->x_range_query);
>>>>   r = new gimple_ranger (use_imm_uses);
>>>>   fun->x_range_query = r;
>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
>>>>   gcc_checking_assert (fun->x_range_query);
>>>>   delete fun->x_range_query;
>>>>   fun->x_range_query = NULL;
>>>> +
>>>> +  bitmap_obstack_release (NULL);
>>> 
>>> Are you not allowed to initialize/use obstacks unless
>>> bitmap_obstack_initialize(NULL) is called?
>> 
>> You can use it with some other obstack, just not the default one.
>> 
>>> If so, wouldn't it be
>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
>>> needs it initialized)?
>> 
>> No, you still need to decide where is the safe point to release it.
>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
>> the default one can nest (has associated nesting counter).  So, the above
>> patch just says that ranger starts using the default obstack in
>> enable_ranger and stops using it in disable_ranger and anything ranger
>> associated in the obstack can be freed at that point.
> 
> I thought ranger never used the default one:
> 
> $ grep bitmap_obstack_initialize *value* *range*
> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
> 
> or even:
> 
> $ grep obstack.*NULL *value* *range*
> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> 
> I'm obviously missing something here.

Look for BITMAP_ALLOC (NULL) in the backtrace in the PR

Richard 


> Aldy
> 

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Aldy Hernandez]

On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
>
>
>
> > Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
> >
> > On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >>
> >> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
> >>>>        PR middle-end/114604
> >>>>        * gimple-range.cc (enable_ranger): Initialize the global
> >>>>        bitmap obstack.
> >>>>        (disable_ranger): Release it.
> >>>> ---
> >>>> gcc/gimple-range.cc | 4 ++++
> >>>> 1 file changed, 4 insertions(+)
> >>>>
> >>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> >>>> index c16b776c1e3..4d3b1ce8588 100644
> >>>> --- a/gcc/gimple-range.cc
> >>>> +++ b/gcc/gimple-range.cc
> >>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
> >>>> {
> >>>>   gimple_ranger *r;
> >>>>
> >>>> +  bitmap_obstack_initialize (NULL);
> >>>> +
> >>>>   gcc_checking_assert (!fun->x_range_query);
> >>>>   r = new gimple_ranger (use_imm_uses);
> >>>>   fun->x_range_query = r;
> >>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
> >>>>   gcc_checking_assert (fun->x_range_query);
> >>>>   delete fun->x_range_query;
> >>>>   fun->x_range_query = NULL;
> >>>> +
> >>>> +  bitmap_obstack_release (NULL);
> >>>
> >>> Are you not allowed to initialize/use obstacks unless
> >>> bitmap_obstack_initialize(NULL) is called?
> >>
> >> You can use it with some other obstack, just not the default one.
> >>
> >>> If so, wouldn't it be
> >>> better to lazily initialize it downstream (bitmap_alloc, or whomever
> >>> needs it initialized)?
> >>
> >> No, you still need to decide where is the safe point to release it.
> >> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
> >> the default one can nest (has associated nesting counter).  So, the above
> >> patch just says that ranger starts using the default obstack in
> >> enable_ranger and stops using it in disable_ranger and anything ranger
> >> associated in the obstack can be freed at that point.
> >
> > I thought ranger never used the default one:
> >
> > $ grep bitmap_obstack_initialize *value* *range*
> > value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >
> > or even:
> >
> > $ grep obstack.*NULL *value* *range*
> > value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> > value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> > value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> > gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
> > value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> >
> > I'm obviously missing something here.
>
> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR

Ahh!  Thanks.

A few default obstack uses snuck in while I wasn't looking.

$ grep BITMAP_ALLOC.*NULL *range*
gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
BITMAP_ALLOC (NULL); }
gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);

I wonder if it would be cleaner to just change these to use named obstacks.

Andrew, is there a reason we were using the default obstack for these?
 For reference, they are  class update_list used in the ranger cache,
ssa_lazy_cache, and dom_ranger.

Aldy

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Richard Biener]

> Am 08.04.2024 um 18:40 schrieb Aldy Hernandez <aldyh@redhat.com>:
> 
> On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
>> 
>> 
>> 
>>>> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
>>> 
>>> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
>>>> 
>>>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
>>>>>>       PR middle-end/114604
>>>>>>       * gimple-range.cc (enable_ranger): Initialize the global
>>>>>>       bitmap obstack.
>>>>>>       (disable_ranger): Release it.
>>>>>> ---
>>>>>> gcc/gimple-range.cc | 4 ++++
>>>>>> 1 file changed, 4 insertions(+)
>>>>>> 
>>>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
>>>>>> index c16b776c1e3..4d3b1ce8588 100644
>>>>>> --- a/gcc/gimple-range.cc
>>>>>> +++ b/gcc/gimple-range.cc
>>>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
>>>>>> {
>>>>>>  gimple_ranger *r;
>>>>>> 
>>>>>> +  bitmap_obstack_initialize (NULL);
>>>>>> +
>>>>>>  gcc_checking_assert (!fun->x_range_query);
>>>>>>  r = new gimple_ranger (use_imm_uses);
>>>>>>  fun->x_range_query = r;
>>>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
>>>>>>  gcc_checking_assert (fun->x_range_query);
>>>>>>  delete fun->x_range_query;
>>>>>>  fun->x_range_query = NULL;
>>>>>> +
>>>>>> +  bitmap_obstack_release (NULL);
>>>>> 
>>>>> Are you not allowed to initialize/use obstacks unless
>>>>> bitmap_obstack_initialize(NULL) is called?
>>>> 
>>>> You can use it with some other obstack, just not the default one.
>>>> 
>>>>> If so, wouldn't it be
>>>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
>>>>> needs it initialized)?
>>>> 
>>>> No, you still need to decide where is the safe point to release it.
>>>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
>>>> the default one can nest (has associated nesting counter).  So, the above
>>>> patch just says that ranger starts using the default obstack in
>>>> enable_ranger and stops using it in disable_ranger and anything ranger
>>>> associated in the obstack can be freed at that point.
>>> 
>>> I thought ranger never used the default one:
>>> 
>>> $ grep bitmap_obstack_initialize *value* *range*
>>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>> 
>>> or even:
>>> 
>>> $ grep obstack.*NULL *value* *range*
>>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
>>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
>>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
>>> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
>>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
>>> 
>>> I'm obviously missing something here.
>> 
>> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR
> 
> Ahh!  Thanks.
> 
> A few default obstack uses snuck in while I wasn't looking.
> 
> $ grep BITMAP_ALLOC.*NULL *range*
> gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
> gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
> BITMAP_ALLOC (NULL); }
> gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);
> 
> I wonder if it would be cleaner to just change these to use named obstacks.

I didn’t find any obvious obstack to use, but sure.  This was the easiest fix ;)

Richard 

> Andrew, is there a reason we were using the default obstack for these?
> For reference, they are  class update_list used in the ranger cache,
> ssa_lazy_cache, and dom_ranger.
> 
> Aldy
> 

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Aldy Hernandez]

BTW, I'm not opposed to this patch.  Thank you for tracking this down,
and feel free to commit as is if y'all PMs agree it's OK.  I just
wanted to know if there's a better way going forward.  I can certainly
put it on my TODO list once stage1 opens again.

And no, there probably isn't an obstack for those classes, but I
wonder if we should have a class local one, as we do for the rest of
the classes.

Aldy

On Mon, Apr 8, 2024 at 7:47 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
>
>
> > Am 08.04.2024 um 18:40 schrieb Aldy Hernandez <aldyh@redhat.com>:
> >
> > On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
> >>
> >>
> >>
> >>>> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
> >>>
> >>> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >>>>
> >>>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
> >>>>>>       PR middle-end/114604
> >>>>>>       * gimple-range.cc (enable_ranger): Initialize the global
> >>>>>>       bitmap obstack.
> >>>>>>       (disable_ranger): Release it.
> >>>>>> ---
> >>>>>> gcc/gimple-range.cc | 4 ++++
> >>>>>> 1 file changed, 4 insertions(+)
> >>>>>>
> >>>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> >>>>>> index c16b776c1e3..4d3b1ce8588 100644
> >>>>>> --- a/gcc/gimple-range.cc
> >>>>>> +++ b/gcc/gimple-range.cc
> >>>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
> >>>>>> {
> >>>>>>  gimple_ranger *r;
> >>>>>>
> >>>>>> +  bitmap_obstack_initialize (NULL);
> >>>>>> +
> >>>>>>  gcc_checking_assert (!fun->x_range_query);
> >>>>>>  r = new gimple_ranger (use_imm_uses);
> >>>>>>  fun->x_range_query = r;
> >>>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
> >>>>>>  gcc_checking_assert (fun->x_range_query);
> >>>>>>  delete fun->x_range_query;
> >>>>>>  fun->x_range_query = NULL;
> >>>>>> +
> >>>>>> +  bitmap_obstack_release (NULL);
> >>>>>
> >>>>> Are you not allowed to initialize/use obstacks unless
> >>>>> bitmap_obstack_initialize(NULL) is called?
> >>>>
> >>>> You can use it with some other obstack, just not the default one.
> >>>>
> >>>>> If so, wouldn't it be
> >>>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
> >>>>> needs it initialized)?
> >>>>
> >>>> No, you still need to decide where is the safe point to release it.
> >>>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
> >>>> the default one can nest (has associated nesting counter).  So, the above
> >>>> patch just says that ranger starts using the default obstack in
> >>>> enable_ranger and stops using it in disable_ranger and anything ranger
> >>>> associated in the obstack can be freed at that point.
> >>>
> >>> I thought ranger never used the default one:
> >>>
> >>> $ grep bitmap_obstack_initialize *value* *range*
> >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>>
> >>> or even:
> >>>
> >>> $ grep obstack.*NULL *value* *range*
> >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> >>> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
> >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> >>>
> >>> I'm obviously missing something here.
> >>
> >> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR
> >
> > Ahh!  Thanks.
> >
> > A few default obstack uses snuck in while I wasn't looking.
> >
> > $ grep BITMAP_ALLOC.*NULL *range*
> > gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
> > gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
> > BITMAP_ALLOC (NULL); }
> > gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);
> >
> > I wonder if it would be cleaner to just change these to use named obstacks.
>
> I didn’t find any obvious obstack to use, but sure.  This was the easiest fix ;)
>
> Richard
>
> > Andrew, is there a reason we were using the default obstack for these?
> > For reference, they are  class update_list used in the ranger cache,
> > ssa_lazy_cache, and dom_ranger.
> >
> > Aldy
> >
>

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 5180 bytes --]

On Tue, 9 Apr 2024, Aldy Hernandez wrote:

> BTW, I'm not opposed to this patch.  Thank you for tracking this down,
> and feel free to commit as is if y'all PMs agree it's OK.  I just
> wanted to know if there's a better way going forward.  I can certainly
> put it on my TODO list once stage1 opens again.
> 
> And no, there probably isn't an obstack for those classes, but I
> wonder if we should have a class local one, as we do for the rest of
> the classes.

OK, I pushed it now, it looks like the GCC 13 branch isn't affected
in obviously the same way (but I didn't try instrumenting there).

Feel free to improve next stage1.

Richard.

> Aldy
> 
> On Mon, Apr 8, 2024 at 7:47 PM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> >
> >
> > > Am 08.04.2024 um 18:40 schrieb Aldy Hernandez <aldyh@redhat.com>:
> > >
> > > On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
> > >>
> > >>
> > >>
> > >>>> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
> > >>>
> > >>> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
> > >>>>
> > >>>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
> > >>>>>>       PR middle-end/114604
> > >>>>>>       * gimple-range.cc (enable_ranger): Initialize the global
> > >>>>>>       bitmap obstack.
> > >>>>>>       (disable_ranger): Release it.
> > >>>>>> ---
> > >>>>>> gcc/gimple-range.cc | 4 ++++
> > >>>>>> 1 file changed, 4 insertions(+)
> > >>>>>>
> > >>>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> > >>>>>> index c16b776c1e3..4d3b1ce8588 100644
> > >>>>>> --- a/gcc/gimple-range.cc
> > >>>>>> +++ b/gcc/gimple-range.cc
> > >>>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
> > >>>>>> {
> > >>>>>>  gimple_ranger *r;
> > >>>>>>
> > >>>>>> +  bitmap_obstack_initialize (NULL);
> > >>>>>> +
> > >>>>>>  gcc_checking_assert (!fun->x_range_query);
> > >>>>>>  r = new gimple_ranger (use_imm_uses);
> > >>>>>>  fun->x_range_query = r;
> > >>>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
> > >>>>>>  gcc_checking_assert (fun->x_range_query);
> > >>>>>>  delete fun->x_range_query;
> > >>>>>>  fun->x_range_query = NULL;
> > >>>>>> +
> > >>>>>> +  bitmap_obstack_release (NULL);
> > >>>>>
> > >>>>> Are you not allowed to initialize/use obstacks unless
> > >>>>> bitmap_obstack_initialize(NULL) is called?
> > >>>>
> > >>>> You can use it with some other obstack, just not the default one.
> > >>>>
> > >>>>> If so, wouldn't it be
> > >>>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
> > >>>>> needs it initialized)?
> > >>>>
> > >>>> No, you still need to decide where is the safe point to release it.
> > >>>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
> > >>>> the default one can nest (has associated nesting counter).  So, the above
> > >>>> patch just says that ranger starts using the default obstack in
> > >>>> enable_ranger and stops using it in disable_ranger and anything ranger
> > >>>> associated in the obstack can be freed at that point.
> > >>>
> > >>> I thought ranger never used the default one:
> > >>>
> > >>> $ grep bitmap_obstack_initialize *value* *range*
> > >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>>
> > >>> or even:
> > >>>
> > >>> $ grep obstack.*NULL *value* *range*
> > >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> > >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> > >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> > >>> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
> > >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> > >>>
> > >>> I'm obviously missing something here.
> > >>
> > >> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR
> > >
> > > Ahh!  Thanks.
> > >
> > > A few default obstack uses snuck in while I wasn't looking.
> > >
> > > $ grep BITMAP_ALLOC.*NULL *range*
> > > gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
> > > gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
> > > BITMAP_ALLOC (NULL); }
> > > gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);
> > >
> > > I wonder if it would be cleaner to just change these to use named obstacks.
> >
> > I didn’t find any obvious obstack to use, but sure.  This was the easiest fix ;)
> >
> > Richard
> >
> > > Andrew, is there a reason we were using the default obstack for these?
> > > For reference, they are  class update_list used in the ranger cache,
> > > ssa_lazy_cache, and dom_ranger.
> > >
> > > Aldy
> > >
> >
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 4629 bytes --]

Hi.

I came around to this, and whipped up the proposed patch.  However, it
does seem a bit verbose, and I'm wondering if it's cleaner to just
leave things as they are.

The attached patch passes tests and there's no difference in
performance.  I am wondering, whether it's better to get rid of
all/most of the local obstacks we use in ranger, and just use the
global (NULL) one?

Thoughts?

Aldy

On Mon, Apr 8, 2024 at 7:47 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
>
>
> > Am 08.04.2024 um 18:40 schrieb Aldy Hernandez <aldyh@redhat.com>:
> >
> > On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
> >>
> >>
> >>
> >>>> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
> >>>
> >>> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >>>>
> >>>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
> >>>>>>       PR middle-end/114604
> >>>>>>       * gimple-range.cc (enable_ranger): Initialize the global
> >>>>>>       bitmap obstack.
> >>>>>>       (disable_ranger): Release it.
> >>>>>> ---
> >>>>>> gcc/gimple-range.cc | 4 ++++
> >>>>>> 1 file changed, 4 insertions(+)
> >>>>>>
> >>>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> >>>>>> index c16b776c1e3..4d3b1ce8588 100644
> >>>>>> --- a/gcc/gimple-range.cc
> >>>>>> +++ b/gcc/gimple-range.cc
> >>>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
> >>>>>> {
> >>>>>>  gimple_ranger *r;
> >>>>>>
> >>>>>> +  bitmap_obstack_initialize (NULL);
> >>>>>> +
> >>>>>>  gcc_checking_assert (!fun->x_range_query);
> >>>>>>  r = new gimple_ranger (use_imm_uses);
> >>>>>>  fun->x_range_query = r;
> >>>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
> >>>>>>  gcc_checking_assert (fun->x_range_query);
> >>>>>>  delete fun->x_range_query;
> >>>>>>  fun->x_range_query = NULL;
> >>>>>> +
> >>>>>> +  bitmap_obstack_release (NULL);
> >>>>>
> >>>>> Are you not allowed to initialize/use obstacks unless
> >>>>> bitmap_obstack_initialize(NULL) is called?
> >>>>
> >>>> You can use it with some other obstack, just not the default one.
> >>>>
> >>>>> If so, wouldn't it be
> >>>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
> >>>>> needs it initialized)?
> >>>>
> >>>> No, you still need to decide where is the safe point to release it.
> >>>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
> >>>> the default one can nest (has associated nesting counter).  So, the above
> >>>> patch just says that ranger starts using the default obstack in
> >>>> enable_ranger and stops using it in disable_ranger and anything ranger
> >>>> associated in the obstack can be freed at that point.
> >>>
> >>> I thought ranger never used the default one:
> >>>
> >>> $ grep bitmap_obstack_initialize *value* *range*
> >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
> >>>
> >>> or even:
> >>>
> >>> $ grep obstack.*NULL *value* *range*
> >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> >>> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
> >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> >>>
> >>> I'm obviously missing something here.
> >>
> >> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR
> >
> > Ahh!  Thanks.
> >
> > A few default obstack uses snuck in while I wasn't looking.
> >
> > $ grep BITMAP_ALLOC.*NULL *range*
> > gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
> > gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
> > BITMAP_ALLOC (NULL); }
> > gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);
> >
> > I wonder if it would be cleaner to just change these to use named obstacks.
>
> I didn’t find any obvious obstack to use, but sure.  This was the easiest fix ;)
>
> Richard
>
> > Andrew, is there a reason we were using the default obstack for these?
> > For reference, they are  class update_list used in the ranger cache,
> > ssa_lazy_cache, and dom_ranger.
> >
> > Aldy
> >
>

[-- Attachment #2: 0001-Avoid-global-bitmap-space-in-ranger.patch --]
[-- Type: text/x-patch, Size: 3975 bytes --]

From aad99849a8ceb41cc8b8e81104b85736ac04a65a Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Wed, 19 Jun 2024 11:42:16 +0200
Subject: [PATCH] Avoid global bitmap space in ranger.

---
 gcc/gimple-range-cache.cc | 6 ++++--
 gcc/gimple-range-cache.h  | 9 +++++++--
 gcc/gimple-range.cc       | 9 +++------
 gcc/gimple-range.h        | 1 +
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index d84fd1ca0e8..6979a14cbaa 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -906,6 +906,7 @@ private:
   vec<int> m_update_list;
   int m_update_head;
   bitmap m_propfail;
+  bitmap_obstack m_bitmaps;
 };
 
 // Create an update list.
@@ -915,7 +916,8 @@ update_list::update_list ()
   m_update_list.create (0);
   m_update_list.safe_grow_cleared (last_basic_block_for_fn (cfun) + 64);
   m_update_head = -1;
-  m_propfail = BITMAP_ALLOC (NULL);
+  bitmap_obstack_initialize (&m_bitmaps);
+  m_propfail = BITMAP_ALLOC (&m_bitmaps);
 }
 
 // Destroy an update list.
@@ -923,7 +925,7 @@ update_list::update_list ()
 update_list::~update_list ()
 {
   m_update_list.release ();
-  BITMAP_FREE (m_propfail);
+  bitmap_obstack_release (&m_bitmaps);
 }
 
 // Add BB to the list of blocks to update, unless it's already in the list.
diff --git a/gcc/gimple-range-cache.h b/gcc/gimple-range-cache.h
index 63410d5437e..0ea34d3f686 100644
--- a/gcc/gimple-range-cache.h
+++ b/gcc/gimple-range-cache.h
@@ -78,8 +78,12 @@ protected:
 class ssa_lazy_cache : public ssa_cache
 {
 public:
-  inline ssa_lazy_cache () { active_p = BITMAP_ALLOC (NULL); }
-  inline ~ssa_lazy_cache () { BITMAP_FREE (active_p); }
+  inline ssa_lazy_cache ()
+  {
+    bitmap_obstack_initialize (&m_bitmaps);
+    active_p = BITMAP_ALLOC (&m_bitmaps);
+  }
+  inline ~ssa_lazy_cache () { bitmap_obstack_release (&m_bitmaps); }
   inline bool empty_p () const { return bitmap_empty_p (active_p); }
   virtual bool has_range (tree name) const;
   virtual bool set_range (tree name, const vrange &r);
@@ -89,6 +93,7 @@ public:
   virtual void clear ();
   void merge (const ssa_lazy_cache &);
 protected:
+  bitmap_obstack m_bitmaps;
   bitmap active_p;
 };
 
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index f3e4ec2d249..1c35f5a4e95 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -701,8 +701,6 @@ enable_ranger (struct function *fun, bool use_imm_uses)
 {
   gimple_ranger *r;
 
-  bitmap_obstack_initialize (NULL);
-
   gcc_checking_assert (!fun->x_range_query);
   r = new gimple_ranger (use_imm_uses);
   fun->x_range_query = r;
@@ -719,8 +717,6 @@ disable_ranger (struct function *fun)
   gcc_checking_assert (fun->x_range_query);
   delete fun->x_range_query;
   fun->x_range_query = NULL;
-
-  bitmap_obstack_release (NULL);
 }
 
 // ------------------------------------------------------------------------
@@ -930,7 +926,8 @@ dom_ranger::dom_ranger () : m_global ()
   m_e0.safe_grow_cleared (last_basic_block_for_fn (cfun));
   m_e1.create (0);
   m_e1.safe_grow_cleared (last_basic_block_for_fn (cfun));
-  m_pop_list = BITMAP_ALLOC (NULL);
+  bitmap_obstack_initialize (&m_bitmaps);
+  m_pop_list = BITMAP_ALLOC (&m_bitmaps);
   if (dump_file && (param_ranger_debug & RANGER_DEBUG_TRACE))
     tracer.enable_trace ();
 }
@@ -945,7 +942,7 @@ dom_ranger::~dom_ranger ()
       fprintf (dump_file, "=========================:\n");
       m_global.dump (dump_file);
     }
-  BITMAP_FREE (m_pop_list);
+  bitmap_obstack_release (&m_bitmaps);
   m_e1.release ();
   m_e0.release ();
   m_freelist.release ();
diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h
index 180090bed15..264413ee458 100644
--- a/gcc/gimple-range.h
+++ b/gcc/gimple-range.h
@@ -124,6 +124,7 @@ protected:
   vec<ssa_lazy_cache *> m_freelist;
   vec<ssa_lazy_cache *> m_e0;
   vec<ssa_lazy_cache *> m_e1;
+  bitmap_obstack m_bitmaps;
   bitmap m_pop_list;
   range_tracer tracer;
 };
-- 
2.45.0

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 5258 bytes --]

On Thu, 20 Jun 2024, Aldy Hernandez wrote:

> Hi.
> 
> I came around to this, and whipped up the proposed patch.  However, it
> does seem a bit verbose, and I'm wondering if it's cleaner to just
> leave things as they are.
> 
> The attached patch passes tests and there's no difference in
> performance.  I am wondering, whether it's better to get rid of
> all/most of the local obstacks we use in ranger, and just use the
> global (NULL) one?
> 
> Thoughts?

It really depends on how much garbage ranger is expected to create
on the obstack - the global obstack is released after each pass.
But ranger instances are also not expected to be created multiple
times each pass, right?

I don't have a strong opinion.

Richard.

> Aldy
> 
> On Mon, Apr 8, 2024 at 7:47 PM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> >
> >
> > > Am 08.04.2024 um 18:40 schrieb Aldy Hernandez <aldyh@redhat.com>:
> > >
> > > On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
> > >>
> > >>
> > >>
> > >>>> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
> > >>>
> > >>> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
> > >>>>
> > >>>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
> > >>>>>>       PR middle-end/114604
> > >>>>>>       * gimple-range.cc (enable_ranger): Initialize the global
> > >>>>>>       bitmap obstack.
> > >>>>>>       (disable_ranger): Release it.
> > >>>>>> ---
> > >>>>>> gcc/gimple-range.cc | 4 ++++
> > >>>>>> 1 file changed, 4 insertions(+)
> > >>>>>>
> > >>>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
> > >>>>>> index c16b776c1e3..4d3b1ce8588 100644
> > >>>>>> --- a/gcc/gimple-range.cc
> > >>>>>> +++ b/gcc/gimple-range.cc
> > >>>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
> > >>>>>> {
> > >>>>>>  gimple_ranger *r;
> > >>>>>>
> > >>>>>> +  bitmap_obstack_initialize (NULL);
> > >>>>>> +
> > >>>>>>  gcc_checking_assert (!fun->x_range_query);
> > >>>>>>  r = new gimple_ranger (use_imm_uses);
> > >>>>>>  fun->x_range_query = r;
> > >>>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
> > >>>>>>  gcc_checking_assert (fun->x_range_query);
> > >>>>>>  delete fun->x_range_query;
> > >>>>>>  fun->x_range_query = NULL;
> > >>>>>> +
> > >>>>>> +  bitmap_obstack_release (NULL);
> > >>>>>
> > >>>>> Are you not allowed to initialize/use obstacks unless
> > >>>>> bitmap_obstack_initialize(NULL) is called?
> > >>>>
> > >>>> You can use it with some other obstack, just not the default one.
> > >>>>
> > >>>>> If so, wouldn't it be
> > >>>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
> > >>>>> needs it initialized)?
> > >>>>
> > >>>> No, you still need to decide where is the safe point to release it.
> > >>>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
> > >>>> the default one can nest (has associated nesting counter).  So, the above
> > >>>> patch just says that ranger starts using the default obstack in
> > >>>> enable_ranger and stops using it in disable_ranger and anything ranger
> > >>>> associated in the obstack can be freed at that point.
> > >>>
> > >>> I thought ranger never used the default one:
> > >>>
> > >>> $ grep bitmap_obstack_initialize *value* *range*
> > >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
> > >>>
> > >>> or even:
> > >>>
> > >>> $ grep obstack.*NULL *value* *range*
> > >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> > >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> > >>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
> > >>> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
> > >>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
> > >>>
> > >>> I'm obviously missing something here.
> > >>
> > >> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR
> > >
> > > Ahh!  Thanks.
> > >
> > > A few default obstack uses snuck in while I wasn't looking.
> > >
> > > $ grep BITMAP_ALLOC.*NULL *range*
> > > gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
> > > gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
> > > BITMAP_ALLOC (NULL); }
> > > gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);
> > >
> > > I wonder if it would be cleaner to just change these to use named obstacks.
> >
> > I didn’t find any obvious obstack to use, but sure.  This was the easiest fix ;)
> >
> > Richard
> >
> > > Andrew, is there a reason we were using the default obstack for these?
> > > For reference, they are  class update_list used in the ranger cache,
> > > ssa_lazy_cache, and dom_ranger.
> > >
> > > Aldy
> > >
> >
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Andrew MacLeod]

On 6/20/24 05:31, Richard Biener wrote:
> On Thu, 20 Jun 2024, Aldy Hernandez wrote:
>
>> Hi.
>>
>> I came around to this, and whipped up the proposed patch.  However, it
>> does seem a bit verbose, and I'm wondering if it's cleaner to just
>> leave things as they are.
>>
>> The attached patch passes tests and there's no difference in
>> performance.  I am wondering, whether it's better to get rid of
>> all/most of the local obstacks we use in ranger, and just use the
>> global (NULL) one?
>>
>> Thoughts?
> It really depends on how much garbage ranger is expected to create
> on the obstack - the global obstack is released after each pass.
> But ranger instances are also not expected to be created multiple
> times each pass, right?


Typically correct.  Although the path ranger also creates  a normal 
ranger,.  Different components also have their own obstacks, mostly 
because they can be used independent of ranger. I didn't want to add 
artificial dependencies just for a obstack sharing.

  I was unaware of how the global one worked at that point. Do they get 
stacked if another global obstack is initialized?   And is there any 
danger in that case twe could accidentally have a sequence like:

   obstack1 created by ranger
   GORI  allocates bitmap from obstack1
   obstack2 created by the pass that decided to use ranger
   GORI allocates bitmap2.. comes from obstack2
   obstack2 destroyed by the pass.
   GORI tries to use bitmap2  .. its now unallocated.

If so, this reeks of the obstack problems we had back in the late 90's 
when obstacks were generally stacked.  Tracking down objects still in 
use from freed obstacks was a nightmare.  That is one of the reason 
general stacked obstacks fell out of favour for a long time, and why i 
only ever use local named one.

  It seems to me that components managing their own obstacks ensures 
this does not happen.

If, however, that is not longer a problem for some reason, then I have 
no strong feelings either way either.

> I don't have a strong opinion.
>
> Richard.
>
>> Aldy
>>
>> On Mon, Apr 8, 2024 at 7:47 PM Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>>
>>>
>>>> Am 08.04.2024 um 18:40 schrieb Aldy Hernandez <aldyh@redhat.com>:
>>>>
>>>> On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
>>>>>
>>>>>
>>>>>>> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
>>>>>> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
>>>>>>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
>>>>>>>>>        PR middle-end/114604
>>>>>>>>>        * gimple-range.cc (enable_ranger): Initialize the global
>>>>>>>>>        bitmap obstack.
>>>>>>>>>        (disable_ranger): Release it.
>>>>>>>>> ---
>>>>>>>>> gcc/gimple-range.cc | 4 ++++
>>>>>>>>> 1 file changed, 4 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
>>>>>>>>> index c16b776c1e3..4d3b1ce8588 100644
>>>>>>>>> --- a/gcc/gimple-range.cc
>>>>>>>>> +++ b/gcc/gimple-range.cc
>>>>>>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
>>>>>>>>> {
>>>>>>>>>   gimple_ranger *r;
>>>>>>>>>
>>>>>>>>> +  bitmap_obstack_initialize (NULL);
>>>>>>>>> +
>>>>>>>>>   gcc_checking_assert (!fun->x_range_query);
>>>>>>>>>   r = new gimple_ranger (use_imm_uses);
>>>>>>>>>   fun->x_range_query = r;
>>>>>>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
>>>>>>>>>   gcc_checking_assert (fun->x_range_query);
>>>>>>>>>   delete fun->x_range_query;
>>>>>>>>>   fun->x_range_query = NULL;
>>>>>>>>> +
>>>>>>>>> +  bitmap_obstack_release (NULL);
>>>>>>>> Are you not allowed to initialize/use obstacks unless
>>>>>>>> bitmap_obstack_initialize(NULL) is called?
>>>>>>> You can use it with some other obstack, just not the default one.
>>>>>>>
>>>>>>>> If so, wouldn't it be
>>>>>>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
>>>>>>>> needs it initialized)?
>>>>>>> No, you still need to decide where is the safe point to release it.
>>>>>>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
>>>>>>> the default one can nest (has associated nesting counter).  So, the above
>>>>>>> patch just says that ranger starts using the default obstack in
>>>>>>> enable_ranger and stops using it in disable_ranger and anything ranger
>>>>>>> associated in the obstack can be freed at that point.
>>>>>> I thought ranger never used the default one:
>>>>>>
>>>>>> $ grep bitmap_obstack_initialize *value* *range*
>>>>>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>>
>>>>>> or even:
>>>>>>
>>>>>> $ grep obstack.*NULL *value* *range*
>>>>>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
>>>>>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
>>>>>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
>>>>>> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
>>>>>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
>>>>>>
>>>>>> I'm obviously missing something here.
>>>>> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR
>>>> Ahh!  Thanks.
>>>>
>>>> A few default obstack uses snuck in while I wasn't looking.
>>>>
>>>> $ grep BITMAP_ALLOC.*NULL *range*
>>>> gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
>>>> gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
>>>> BITMAP_ALLOC (NULL); }
>>>> gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);
>>>>
>>>> I wonder if it would be cleaner to just change these to use named obstacks.
>>> I didn’t find any obvious obstack to use, but sure.  This was the easiest fix ;)
>>>
>>> Richard
>>>
>>>> Andrew, is there a reason we were using the default obstack for these?
>>>> For reference, they are  class update_list used in the ranger cache,
>>>> ssa_lazy_cache, and dom_ranger.
>>>>
>>>> Aldy
>>>>

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Richard Biener]

> Am 20.06.2024 um 16:05 schrieb Andrew MacLeod <amacleod@redhat.com>:
> 
> 
>> On 6/20/24 05:31, Richard Biener wrote:
>>> On Thu, 20 Jun 2024, Aldy Hernandez wrote:
>>> 
>>> Hi.
>>> 
>>> I came around to this, and whipped up the proposed patch.  However, it
>>> does seem a bit verbose, and I'm wondering if it's cleaner to just
>>> leave things as they are.
>>> 
>>> The attached patch passes tests and there's no difference in
>>> performance.  I am wondering, whether it's better to get rid of
>>> all/most of the local obstacks we use in ranger, and just use the
>>> global (NULL) one?
>>> 
>>> Thoughts?
>> It really depends on how much garbage ranger is expected to create
>> on the obstack - the global obstack is released after each pass.
>> But ranger instances are also not expected to be created multiple
>> times each pass, right?
> 
> 
> Typically correct.  Although the path ranger also creates  a normal ranger,.  Different components also have their own obstacks, mostly because they can be used independent of ranger. I didn't want to add artificial dependencies just for a obstack sharing.
> 
>  I was unaware of how the global one worked at that point. Do they get stacked if another global obstack is initialized?   And is there any danger in that case twe could accidentally have a sequence like:
> 
>   obstack1 created by ranger
>   GORI  allocates bitmap from obstack1
>   obstack2 created by the pass that decided to use ranger
>   GORI allocates bitmap2.. comes from obstack2
>   obstack2 destroyed by the pass.
>   GORI tries to use bitmap2  .. its now unallocated.
> 
> If so, this reeks of the obstack problems we had back in the late 90's when obstacks were generally stacked.  Tracking down objects still in use from freed obstacks was a nightmare.  That is one of the reason general stacked obstacks fell out of favour for a long time, and why i only ever use local named one.
> 
>  It seems to me that components managing their own obstacks ensures this does not happen.
> 
> If, however, that is not longer a problem for some reason, then I have no strong feelings either way either.

The global obstack is special, it’s init keeps a reference count.  So yes, a local obstack is cleaner.

Richard 

>> I don't have a strong opinion.
>> 
>> Richard.
>> 
>>> Aldy
>>> 
>>> On Mon, Apr 8, 2024 at 7:47 PM Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> 
>>>> 
>>>>> Am 08.04.2024 um 18:40 schrieb Aldy Hernandez <aldyh@redhat.com>:
>>>>> 
>>>>> On Mon, Apr 8, 2024 at 6:29 PM Richard Biener <rguenther@suse.de> wrote:
>>>>>> 
>>>>>> 
>>>>>>>> Am 08.04.2024 um 18:09 schrieb Aldy Hernandez <aldyh@redhat.com>:
>>>>>>> On Mon, Apr 8, 2024 at 5:54 PM Jakub Jelinek <jakub@redhat.com> wrote:
>>>>>>>> On Mon, Apr 08, 2024 at 05:40:23PM +0200, Aldy Hernandez wrote:
>>>>>>>>>>       PR middle-end/114604
>>>>>>>>>>       * gimple-range.cc (enable_ranger): Initialize the global
>>>>>>>>>>       bitmap obstack.
>>>>>>>>>>       (disable_ranger): Release it.
>>>>>>>>>> ---
>>>>>>>>>> gcc/gimple-range.cc | 4 ++++
>>>>>>>>>> 1 file changed, 4 insertions(+)
>>>>>>>>>> 
>>>>>>>>>> diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
>>>>>>>>>> index c16b776c1e3..4d3b1ce8588 100644
>>>>>>>>>> --- a/gcc/gimple-range.cc
>>>>>>>>>> +++ b/gcc/gimple-range.cc
>>>>>>>>>> @@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
>>>>>>>>>> {
>>>>>>>>>>  gimple_ranger *r;
>>>>>>>>>> 
>>>>>>>>>> +  bitmap_obstack_initialize (NULL);
>>>>>>>>>> +
>>>>>>>>>>  gcc_checking_assert (!fun->x_range_query);
>>>>>>>>>>  r = new gimple_ranger (use_imm_uses);
>>>>>>>>>>  fun->x_range_query = r;
>>>>>>>>>> @@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
>>>>>>>>>>  gcc_checking_assert (fun->x_range_query);
>>>>>>>>>>  delete fun->x_range_query;
>>>>>>>>>>  fun->x_range_query = NULL;
>>>>>>>>>> +
>>>>>>>>>> +  bitmap_obstack_release (NULL);
>>>>>>>>> Are you not allowed to initialize/use obstacks unless
>>>>>>>>> bitmap_obstack_initialize(NULL) is called?
>>>>>>>> You can use it with some other obstack, just not the default one.
>>>>>>>> 
>>>>>>>>> If so, wouldn't it be
>>>>>>>>> better to lazily initialize it downstream (bitmap_alloc, or whomever
>>>>>>>>> needs it initialized)?
>>>>>>>> No, you still need to decide where is the safe point to release it.
>>>>>>>> Unlike the non-default bitmap_obstack_initialize/bitmap_obstack_release,
>>>>>>>> the default one can nest (has associated nesting counter).  So, the above
>>>>>>>> patch just says that ranger starts using the default obstack in
>>>>>>>> enable_ranger and stops using it in disable_ranger and anything ranger
>>>>>>>> associated in the obstack can be freed at that point.
>>>>>>> I thought ranger never used the default one:
>>>>>>> 
>>>>>>> $ grep bitmap_obstack_initialize *value* *range*
>>>>>>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>>> value-relation.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>>> gimple-range-cache.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>>> gimple-range-gori.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>>> gimple-range-infer.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>>> gimple-range-phi.cc:  bitmap_obstack_initialize (&m_bitmaps);
>>>>>>> 
>>>>>>> or even:
>>>>>>> 
>>>>>>> $ grep obstack.*NULL *value* *range*
>>>>>>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
>>>>>>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
>>>>>>> value-relation.cc:  obstack_free (&m_chain_obstack, NULL);
>>>>>>> gimple-range-infer.cc:  obstack_free (&m_list_obstack, NULL);
>>>>>>> value-range-storage.cc:    obstack_free (&m_obstack, NULL);
>>>>>>> 
>>>>>>> I'm obviously missing something here.
>>>>>> Look for BITMAP_ALLOC (NULL) in the backtrace in the PR
>>>>> Ahh!  Thanks.
>>>>> 
>>>>> A few default obstack uses snuck in while I wasn't looking.
>>>>> 
>>>>> $ grep BITMAP_ALLOC.*NULL *range*
>>>>> gimple-range-cache.cc:  m_propfail = BITMAP_ALLOC (NULL);
>>>>> gimple-range-cache.h:  inline ssa_lazy_cache () { active_p =
>>>>> BITMAP_ALLOC (NULL); }
>>>>> gimple-range.cc:  m_pop_list = BITMAP_ALLOC (NULL);
>>>>> 
>>>>> I wonder if it would be cleaner to just change these to use named obstacks.
>>>> I didn’t find any obvious obstack to use, but sure.  This was the easiest fix ;)
>>>> 
>>>> Richard
>>>> 
>>>>> Andrew, is there a reason we were using the default obstack for these?
>>>>> For reference, they are  class update_list used in the ranger cache,
>>>>> ssa_lazy_cache, and dom_ranger.
>>>>> 
>>>>> Aldy
>>>>> 
> 

Re: [PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 2556 bytes --]



On 6/20/24 4:36 PM, Richard Biener wrote:
> 
> 
>> Am 20.06.2024 um 16:05 schrieb Andrew MacLeod <amacleod@redhat.com>:
>>
>> 
>>> On 6/20/24 05:31, Richard Biener wrote:
>>>> On Thu, 20 Jun 2024, Aldy Hernandez wrote:
>>>>
>>>> Hi.
>>>>
>>>> I came around to this, and whipped up the proposed patch.  However, it
>>>> does seem a bit verbose, and I'm wondering if it's cleaner to just
>>>> leave things as they are.
>>>>
>>>> The attached patch passes tests and there's no difference in
>>>> performance.  I am wondering, whether it's better to get rid of
>>>> all/most of the local obstacks we use in ranger, and just use the
>>>> global (NULL) one?
>>>>
>>>> Thoughts?
>>> It really depends on how much garbage ranger is expected to create
>>> on the obstack - the global obstack is released after each pass.
>>> But ranger instances are also not expected to be created multiple
>>> times each pass, right?
>>
>>
>> Typically correct.  Although the path ranger also creates  a normal ranger,.  Different components also have their own obstacks, mostly because they can be used independent of ranger. I didn't want to add artificial dependencies just for a obstack sharing.
>>
>>   I was unaware of how the global one worked at that point. Do they get stacked if another global obstack is initialized?   And is there any danger in that case twe could accidentally have a sequence like:
>>
>>    obstack1 created by ranger
>>    GORI  allocates bitmap from obstack1
>>    obstack2 created by the pass that decided to use ranger
>>    GORI allocates bitmap2.. comes from obstack2
>>    obstack2 destroyed by the pass.
>>    GORI tries to use bitmap2  .. its now unallocated.
>>
>> If so, this reeks of the obstack problems we had back in the late 90's when obstacks were generally stacked.  Tracking down objects still in use from freed obstacks was a nightmare.  That is one of the reason general stacked obstacks fell out of favour for a long time, and why i only ever use local named one.
>>
>>   It seems to me that components managing their own obstacks ensures this does not happen.
>>
>> If, however, that is not longer a problem for some reason, then I have no strong feelings either way either.
> 
> The global obstack is special, it’s init keeps a reference count.  So yes, a local obstack is cleaner.

Ok, since a local obstack is cleaner and a global one has the potential 
to introduce subtle bugs, I have rebased the patch against current 
mainline and will commit the attached if it passes tests.

Thanks for everyone's feedback.
Aldy

[-- Attachment #2: 0001-Avoid-global-bitmap-space-in-ranger.patch --]
[-- Type: text/x-patch, Size: 3201 bytes --]

From cd7b03ba43a74ae808a3005ff0e66cd8fabdaea3 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Wed, 19 Jun 2024 11:42:16 +0200
Subject: [PATCH] Avoid global bitmap space in ranger.

gcc/ChangeLog:

	* gimple-range-cache.cc (update_list::update_list): Add m_bitmaps.
	(update_list::~update_list): Initialize m_bitmaps.
	* gimple-range-cache.h (ssa_lazy_cache): Add m_bitmaps.
	* gimple-range.cc (enable_ranger): Remove global bitmap
	initialization.
	(disable_ranger): Remove global bitmap release.
---
 gcc/gimple-range-cache.cc | 6 ++++--
 gcc/gimple-range-cache.h  | 9 +++++++--
 gcc/gimple-range.cc       | 4 ----
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index d84fd1ca0e8..6979a14cbaa 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -906,6 +906,7 @@ private:
   vec<int> m_update_list;
   int m_update_head;
   bitmap m_propfail;
+  bitmap_obstack m_bitmaps;
 };
 
 // Create an update list.
@@ -915,7 +916,8 @@ update_list::update_list ()
   m_update_list.create (0);
   m_update_list.safe_grow_cleared (last_basic_block_for_fn (cfun) + 64);
   m_update_head = -1;
-  m_propfail = BITMAP_ALLOC (NULL);
+  bitmap_obstack_initialize (&m_bitmaps);
+  m_propfail = BITMAP_ALLOC (&m_bitmaps);
 }
 
 // Destroy an update list.
@@ -923,7 +925,7 @@ update_list::update_list ()
 update_list::~update_list ()
 {
   m_update_list.release ();
-  BITMAP_FREE (m_propfail);
+  bitmap_obstack_release (&m_bitmaps);
 }
 
 // Add BB to the list of blocks to update, unless it's already in the list.
diff --git a/gcc/gimple-range-cache.h b/gcc/gimple-range-cache.h
index 63410d5437e..0ea34d3f686 100644
--- a/gcc/gimple-range-cache.h
+++ b/gcc/gimple-range-cache.h
@@ -78,8 +78,12 @@ protected:
 class ssa_lazy_cache : public ssa_cache
 {
 public:
-  inline ssa_lazy_cache () { active_p = BITMAP_ALLOC (NULL); }
-  inline ~ssa_lazy_cache () { BITMAP_FREE (active_p); }
+  inline ssa_lazy_cache ()
+  {
+    bitmap_obstack_initialize (&m_bitmaps);
+    active_p = BITMAP_ALLOC (&m_bitmaps);
+  }
+  inline ~ssa_lazy_cache () { bitmap_obstack_release (&m_bitmaps); }
   inline bool empty_p () const { return bitmap_empty_p (active_p); }
   virtual bool has_range (tree name) const;
   virtual bool set_range (tree name, const vrange &r);
@@ -89,6 +93,7 @@ public:
   virtual void clear ();
   void merge (const ssa_lazy_cache &);
 protected:
+  bitmap_obstack m_bitmaps;
   bitmap active_p;
 };
 
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 50448ef81a2..5df649e268c 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -681,8 +681,6 @@ enable_ranger (struct function *fun, bool use_imm_uses)
 {
   gimple_ranger *r;
 
-  bitmap_obstack_initialize (NULL);
-
   gcc_checking_assert (!fun->x_range_query);
   r = new gimple_ranger (use_imm_uses);
   fun->x_range_query = r;
@@ -699,8 +697,6 @@ disable_ranger (struct function *fun)
   gcc_checking_assert (fun->x_range_query);
   delete fun->x_range_query;
   fun->x_range_query = NULL;
-
-  bitmap_obstack_release (NULL);
 }
 
 // ------------------------------------------------------------------------
-- 
2.45.0

[PATCH] middle-end/114604 - ranger allocates bitmap without initialized obstack

[Richard Biener]

The following fixes ranger bitmap allocation when invoked from IPA
context where the global bitmap obstack possibly isn't initialized.
Instead of trying to use one of the ranger obstacks the following
simply initializes the global bitmap obstack around an active ranger.

Bootstrapped and tested on x86_64-unknown-linux-gnu with bitmap_alloc
instrumentation (all ICEs gone, so ranger was the only offender).

OK?

Thanks,
Richard.

	PR middle-end/114604
	* gimple-range.cc (enable_ranger): Initialize the global
	bitmap obstack.
	(disable_ranger): Release it.
---
 gcc/gimple-range.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index c16b776c1e3..4d3b1ce8588 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -689,6 +689,8 @@ enable_ranger (struct function *fun, bool use_imm_uses)
 {
   gimple_ranger *r;
 
+  bitmap_obstack_initialize (NULL);
+
   gcc_checking_assert (!fun->x_range_query);
   r = new gimple_ranger (use_imm_uses);
   fun->x_range_query = r;
@@ -705,6 +707,8 @@ disable_ranger (struct function *fun)
   gcc_checking_assert (fun->x_range_query);
   delete fun->x_range_query;
   fun->x_range_query = NULL;
+
+  bitmap_obstack_release (NULL);
 }
 
 // ------------------------------------------------------------------------
-- 
2.35.3