On 03/22/2016 03:37 AM, Richard Biener wrote:
On Mon, Mar 21, 2016 at 9:32 PM, Jeff Law <l...@redhat.com> wrote:
On 03/21/2016 01:10 PM, Bernd Schmidt wrote:

On 03/21/2016 08:06 PM, Jeff Law wrote:


As noted last week, find_removable_extensions initializes several
bitmaps, but doesn't clear them.

This is not strictly a leak as the GC system should find dead data, but
it's better to go ahead and clear the bitmaps.  That releases the
elements back to the cache and presumably makes things easier for the GC
system as well.

Bootstrapped and regression tested on x86_64-linux-gnu.

OK for the trunk?


Looks like they don't leak anywhere, so ok. Probably ok even to install
it now but maybe stage1 would be better timing.

I don't mind waiting for the next stage1, this is a pretty minor issue.

It's ok at this stage as it will also fix -fmem-report.  Please also move
the thing back to heap, see below.

Btw we should disallow bitmap_initialize (&x, NULL) as it does not do
the same thing as BITMAP_ALLOC (NULL), it does the same thing
as BITMAP_ALLOC_GC ().  Thus I'd rather have a bitmap_initialize_gc (&x)
and a bitmap_initialize (&x, NULL) that ends up using the global
bitmap obstack.  No idea where REE came from history wise.

A grep shows only

ira.c:  bitmap_initialize (&seen_insns, NULL);
ree.c:  bitmap_initialize (&init, NULL);
ree.c:  bitmap_initialize (&kill, NULL);
ree.c:  bitmap_initialize (&gen, NULL);
ree.c:  bitmap_initialize (&tmp, NULL);
It's more than that. Sadly folks have passed in "0" instead of NULL in various places.

./haifa-sched.c:  bitmap_initialize (&processed, 0);
./haifa-sched.c:  bitmap_initialize (&processed, 0);
./haifa-sched.c:  bitmap_initialize (&in_ready, 0);
./sched-ebb.c:  bitmap_initialize (&dont_calc_deps, 0);
./sched-rgn.c:  bitmap_initialize (&not_in_df, 0);
./testsuite/gcc.dg/pr45352.c:      bitmap_initialize_stat (0);
./ira.c:  bitmap_initialize (&interesting, 0);
./ira.c:  bitmap_initialize (&live, 0);
./ira.c:  bitmap_initialize (&used, 0);
./ira.c:  bitmap_initialize (&set, 0);
./ira.c:  bitmap_initialize (&unusable_as_input, 0);
./ira.c:      bitmap_initialize (local, 0);
./ira.c:      bitmap_initialize (transp, 0);
./ira.c:      bitmap_initialize (moveable, 0);
./ira.c:  bitmap_initialize (&need_new, 0);
./ira.c:  bitmap_initialize (&reachable, 0);
./sel-sched.c:  bitmap_initialize (forced_ebb_heads, 0);
./sched-deps.c:   bitmap_initialize (&true_dependency_cache[i], 0);
./sched-deps.c:   bitmap_initialize (&output_dependency_cache[i], 0);
./sched-deps.c:   bitmap_initialize (&anti_dependency_cache[i], 0);
./sched-deps.c:   bitmap_initialize (&control_dependency_cache[i], 0);
./sched-deps.c:            bitmap_initialize (&spec_dependency_cache[i], 0);


btw, so please consider simply changing bitmap_initialize behavior.  The IRA
use also should use the global bitmap obstack as users around that use
use BITMAP_ALLOC (NULL).  [use a default arg for 'obstack' if possible,
you have to verify it works with/without --enable-gather-detailed-mem-stats]
The problem is ensuring that allocating off the default bitmap obstack is appropriate for all those uses.

I'm tempted to change them all to NULL. Then iterate one by one on to ensure we're routing to gc vs the default bitmap obstack as appropriate and that we're calling bitmap_clear as appropriate.

Once we've fixed all of 'em, we simply assert that bitmap_initialize is never passed NULL and avoid getting in this situation again in the future.

Thoughts?
jeff


Reply via email to