From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 93944 invoked by alias); 11 Oct 2016 15:15:28 -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 93922 invoked by uid 89); 11 Oct 2016 15:15:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=3.0 required=5.0 tests=BAYES_00,MEDICAL_SUBJECT,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=H*M:109, Unlock, read-md.h, invasive 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 ESMTP; Tue, 11 Oct 2016 15:15:15 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (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 CBBC57EA92; Tue, 11 Oct 2016 15:15:14 +0000 (UTC) Received: from vpn-236-115.phx2.redhat.com (vpn-236-115.phx2.redhat.com [10.3.236.115]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9BFFDFA030869; Tue, 11 Oct 2016 11:15:14 -0400 Message-ID: <1476198913.30303.109.camel@redhat.com> Subject: Re: [PATCH 01/16] read-md.c: Add various cleanups to ~rtx_reader From: David Malcolm To: Bernd Schmidt , gcc-patches@gcc.gnu.org Cc: Richard Sandiford Date: Tue, 11 Oct 2016 15:15:00 -0000 In-Reply-To: <27a0ec4d-4df7-ba5e-7235-34578e885ead@redhat.com> References: <1475684110-2521-1-git-send-email-dmalcolm@redhat.com> <1475684110-2521-2-git-send-email-dmalcolm@redhat.com> <27a0ec4d-4df7-ba5e-7235-34578e885ead@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00742.txt.bz2 On Wed, 2016-10-05 at 17:51 +0200, Bernd Schmidt wrote: > On 10/05/2016 06:14 PM, David Malcolm wrote: > > The selftests for the RTL frontend require supporting multiple > > reader instances being alive one after another in-process, so > > this lack of cleanup would become a leak. > > > + /* Initialize global data. */ > > + obstack_init (&string_obstack); > > + ptr_locs = htab_create (161, leading_ptr_hash, leading_ptr_eq_p, > > 0); > > + obstack_init (&ptr_loc_obstack); > > + joined_conditions = htab_create (161, leading_ptr_hash, > > leading_ptr_eq_p, 0); > > + obstack_init (&joined_conditions_obstack); > > + md_constants = htab_create (31, leading_string_hash, > > + leading_string_eq_p, (htab_del) 0); > > + enum_types = htab_create (31, leading_string_hash, > > + leading_string_eq_p, (htab_del) 0); > > + > > + /* Unlock the stdio streams. */ > > + unlock_std_streams (); > > Hmm, but these are global statics. Shouldn't they first be moved to > become class members? [CCing Richard Sandiford] I tried to move these into class rtx_reader, but doing so rapidly became quite invasive, with many of functions in the gen* tools becoming methods. Arguably that would be a good thing, but there are a couple of issues: (a) some of these functions take "vec" arguments; moving them from static functions to being class methods requires that vec.h has been included when the relevant class decl is parsed. (b) rtx_reader turns into a bug dumping ground of methods, for the union of all of the various gen* tools. One way to alleviate these issues would be to do the split of rtx_reader into base_rtx_reader vs rtx_reader from patch 9 of the kit: https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00273.html and perhaps to split out part of read-md.h into a new read-rtl.h. Before I reorganize the patches, does this approach sound reasonable? Alternatively, a less invasive approach is to have an accessor for these fields, so that things using them can get at them via the rtx_reader_ptr singleton e.g.: void grow_string_obstack (char ch) { obstack_1grow (rtx_reader_ptr->get_string_obstack (), ch); } and similar. Thoughts? Dave