From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6670 invoked by alias); 4 Nov 2014 16:16:45 -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 6649 invoked by uid 89); 4 Nov 2014 16:16:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients 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; Tue, 04 Nov 2014 16:16:43 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sA4GGevj026875 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 4 Nov 2014 11:16:41 -0500 Received: from [10.3.236.68] (vpn-236-68.phx2.redhat.com [10.3.236.68]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sA4GGdi7013061; Tue, 4 Nov 2014 11:16:40 -0500 Message-ID: <1415117545.29411.90.camel@surprise> Subject: Re: [PATCH 12/27] New file: gcc/jit/jit-recording.h From: David Malcolm To: Jeff Law Cc: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Date: Tue, 04 Nov 2014 16:16:00 -0000 In-Reply-To: <5457F348.2000009@redhat.com> References: <1414774977-25605-1-git-send-email-dmalcolm@redhat.com> <1414774977-25605-13-git-send-email-dmalcolm@redhat.com> <5457F348.2000009@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-11/txt/msg00194.txt.bz2 On Mon, 2014-11-03 at 14:27 -0700, Jeff Law wrote: > On 10/31/14 11:02, David Malcolm wrote: > > This file declares the gcc::jit::recording internal API, so that > > libgccjit.c can record the calls that are made to the public API, for > > later playback by the dummy frontend. > > > > gcc/jit/ > > * jit-recording.h: New. > > --- > > gcc/jit/jit-recording.h | 1593 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 1593 insertions(+) > > create mode 100644 gcc/jit/jit-recording.h > > > > diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h > > new file mode 100644 > > index 0000000..bb1a2ee > > --- /dev/null > > +++ b/gcc/jit/jit-recording.h > [ ... ] > > > + > > +private: > > + void validate (); > So give the complexities in interfacing with the guts of GCC, would it > make sense to expose the validate method? Most of the error-checking in the API happens in the API calls in libgccjit.c, testing that the individual pieces are sane. The validate method tests the things that can only be verified "as a whole", when the context is about to be compiled: are there unreachable blocks? is every block terminated? etc I can't quite see why client code might want to perform the latter kind of validation without actually doing a compile, so I don't plan to expose this at this time. It's trivial to do so if someone needs it. > > +/* or just use std::string? */ > > +class string : public memento > Is there some reason not to use std::string? I really like using > standard components rather than rolling our own. I think I was trying to avoid std::string for some reason, but I'm not quite sure why, perhaps out of a misremembered idea that libstdc++ was verboten (I currently use std:: in one place, in jit-playback.h, where a playback::context has a: vec > m_cached_locations; ). In any case recording::string is an implementation detail hidden within the library. It is a recording::memento and hence has the same lifetime as the recording::context. I can't think of a reason off the top of my head why a std::string wouldn't work instead, but the existing code works, and has been through a fair amount of debugging. One thing I do make use of is that it's a pointer, for this field within recording::memento: string *m_debug_string; so that I can use NULL for the common case of "no debug string has been built for this thing yet" - though I suspect I could use a std::string * for that. > OK for the trunk. Thanks.