From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 97165 invoked by alias); 28 Apr 2016 19:35:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 97150 invoked by uid 89); 28 Apr 2016 19:35:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=routing, IRA, Looks X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 28 Apr 2016 19:35:19 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A7F5664469; Thu, 28 Apr 2016 19:35:17 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-93.phx2.redhat.com [10.3.113.93]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3SJZHjv019553; Thu, 28 Apr 2016 15:35:17 -0400 Subject: Re: [RFA][PATCH] Adding missing calls to bitmap_clear To: Richard Biener References: <56F0461C.3070601@redhat.com> <56F0470F.6080804@redhat.com> <56F05A74.2080200@redhat.com> Cc: Bernd Schmidt , GCC Patches From: Jeff Law Message-ID: Date: Thu, 28 Apr 2016 19:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-04/txt/msg01900.txt.bz2 On 03/22/2016 03:37 AM, Richard Biener wrote: > On Mon, Mar 21, 2016 at 9:32 PM, Jeff Law 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 (¬_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