From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62043 invoked by alias); 17 Oct 2016 11:37:31 -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 62034 invoked by uid 89); 17 Oct 2016 11:37:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=4.8 required=5.0 tests=AWL,BAYES_20,FREEMAIL_FROM,MEDICAL_SUBJECT,RCVD_IN_DNSWL_LOW,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=*loc, sk:obstack, htab_t, 1167 X-HELO: mail-qt0-f178.google.com Received: from mail-qt0-f178.google.com (HELO mail-qt0-f178.google.com) (209.85.216.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Oct 2016 11:37:20 +0000 Received: by mail-qt0-f178.google.com with SMTP id q7so116599203qtq.1 for ; Mon, 17 Oct 2016 04:37:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version; bh=vhMQQYCn8VE0yo0chN2Z/Pxo20NeuFbitJB+aNEUwso=; b=eLewTrzR4tzkasrVAQhk+FFjjQKYitMUr18SvXX8lZtPO6x5/JNtfgarBEtgJ4YgXV q0eUSN0cK2afuzfedX6mH/i3/Eurs2PXZ/+7HOZn0Ey+cTAdGAVrh4KebGCGXiqu+L2T s+jxtiMsRnSVu8E5ra8gxb11X1FnBn3AzmYliG4Fd1W0YhFNQiMQaOR8PwOV/0GQClBu V/Jwru8iCriQX+gBIHgw642gzCvhD3Db2lOVSZsWxvcWEnnZLgz1pJH3c72do1qAQYzq hcO1PJH3qSsiC2DUjahzpuQFbl9l7t7KeaseQztaamMIat64QPGzLkZ3kJoqtSCgV0+P 8M0A== X-Gm-Message-State: AA6/9RmeDJci/tXwQnFRiOpsNo7LZ4CTDzxAje77vo1mtyefNMDsagwcDNzQoLz+X2NqNA== X-Received: by 10.28.56.1 with SMTP id f1mr7642116wma.98.1476704237436; Mon, 17 Oct 2016 04:37:17 -0700 (PDT) Received: from localhost ([95.144.14.157]) by smtp.googlemail.com with ESMTPSA id w1sm1763311wje.36.2016.10.17.04.37.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 17 Oct 2016 04:37:16 -0700 (PDT) From: Richard Sandiford To: David Malcolm Mail-Followup-To: David Malcolm ,Bernd Schmidt , gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: Bernd Schmidt , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] read-md.c: Move various state to within class rtx_reader References: <87mvi9dydt.fsf@googlemail.com> <1476468970-12787-1-git-send-email-dmalcolm@redhat.com> Date: Mon, 17 Oct 2016 11:37:00 -0000 In-Reply-To: <1476468970-12787-1-git-send-email-dmalcolm@redhat.com> (David Malcolm's message of "Fri, 14 Oct 2016 14:16:10 -0400") Message-ID: <87oa2jnr4j.fsf@googlemail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2016-10/txt/msg01284.txt.bz2 David Malcolm writes: > @@ -2388,8 +2389,10 @@ gen_mnemonic_setattr (htab_t mnemonic_htab, rtx insn) > static int > mnemonic_htab_callback (void **slot, void *info ATTRIBUTE_UNUSED) > { > - obstack_grow (&string_obstack, (char*)*slot, strlen ((char*)*slot)); > - obstack_1grow (&string_obstack, ','); > + struct obstack *string_obstack = rtx_reader_ptr->get_string_obstack (); > + > + obstack_grow (string_obstack, (char*)*slot, strlen ((char*)*slot)); > + obstack_1grow (string_obstack, ','); The old code was missing a space after the cast. Might be good to add it while you're there. > @@ -2470,8 +2474,8 @@ gen_mnemonic_attr (void) > htab_traverse (mnemonic_htab, mnemonic_htab_callback, NULL); > > /* Replace the last ',' with the zero end character. */ > - *((char *)obstack_next_free (&string_obstack) - 1) = '\0'; > - XSTR (mnemonic_attr, 1) = XOBFINISH (&string_obstack, char *); > + *((char *)obstack_next_free (string_obstack) - 1) = '\0'; > + XSTR (mnemonic_attr, 1) = XOBFINISH (string_obstack, char *); Same here. > @@ -129,9 +105,9 @@ get_md_ptr_loc (const void *ptr) > void > copy_md_ptr_loc (const void *new_ptr, const void *old_ptr) > { > - const struct ptr_loc *loc = get_md_ptr_loc (old_ptr); > + const struct ptr_loc *loc = rtx_reader_ptr->get_md_ptr_loc (old_ptr); > if (loc != 0) > - set_md_ptr_loc (new_ptr, loc->filename, loc->lineno); > + rtx_reader_ptr->set_md_ptr_loc (new_ptr, loc->filename, loc->lineno); > } I suppose it's bikeshedding, but since you need access to an rtx_reader to get and set the locations, it feels like the rtx_reader should be a parameter here too. > @@ -140,7 +116,7 @@ copy_md_ptr_loc (const void *new_ptr, const void *old_ptr) > void > fprint_md_ptr_loc (FILE *outf, const void *ptr) > { > - const struct ptr_loc *loc = get_md_ptr_loc (ptr); > + const struct ptr_loc *loc = rtx_reader_ptr->get_md_ptr_loc (ptr); > if (loc != 0) > fprintf (outf, "#line %d \"%s\"\n", loc->lineno, loc->filename); > } Same here. > +/* As rtx_reader::join_c_conditions above, but call it on the singleton > + rtx_reader instance. */ > + > +const char * > +join_c_conditions (const char *cond1, const char *cond2) > +{ > + return rtx_reader_ptr->join_c_conditions (cond1, cond2); > +} > + I think it'd be better to have the callers use the rtx_reader_ptr explicitly. In general I think it'd be better if the read-md.h interface itself didn't rely on there being a singleton, other than providing the rtx_reader_ptr variable. It would then be easy to allow non-singleton uses in future, without having to update gen* calls a second time. > @@ -204,6 +189,15 @@ fprint_c_condition (FILE *outf, const char *cond) > } > } > > +/* As rtx_reader::fprint_c_condition above, but call it on the singleton > + rtx_reader instance. */ > + > +void > +fprint_c_condition (FILE *outf, const char *cond) > +{ > + rtx_reader_ptr->fprint_c_condition (outf, cond); > +} > + > /* Special fprint_c_condition for writing to STDOUT. */ > > void Another instance of the above. > @@ -808,7 +802,7 @@ handle_constants (void) > void > traverse_md_constants (htab_trav callback, void *info) > { > - htab_traverse (md_constants, callback, info); > + htab_traverse (rtx_reader_ptr->get_md_constants (), callback, info); > } Here too. I assume you didn't turn the functions above into member functions because they aren't primitives. OTOH it might seem inconsistent to have things like traverse_enum_types as member functions but traverse_md_constants not. Thanks, Richard