From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18447 invoked by alias); 4 Nov 2013 22:28:23 -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 18434 invoked by uid 89); 4 Nov 2013 22:28:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Nov 2013 22:28:20 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rA4MSDD0006013 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 4 Nov 2013 17:28:13 -0500 Received: from [10.10.53.219] (vpn-53-219.rdu2.redhat.com [10.10.53.219]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rA4MSCEd015219; Mon, 4 Nov 2013 17:28:12 -0500 Message-ID: <52781F7C.6090106@redhat.com> Date: Mon, 04 Nov 2013 22:31:00 -0000 From: Andrew MacLeod User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8 MIME-Version: 1.0 To: David Malcolm CC: Jakub Jelinek , gcc-patches@gcc.gnu.org Subject: Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3) References: <5271CBF9.2070005@redhat.com> <1383236801-13234-1-git-send-email-dmalcolm@redhat.com> <52741EE2.3030100@redhat.com> <20131101214148.GF27813@tucnak.zalov.cz> <52742162.2010009@redhat.com> <20131101215709.GG27813@tucnak.zalov.cz> <1383346704.5282.44.camel@surprise> <52779EF5.8000401@redhat.com> <1383601413.5282.62.camel@surprise> <1383602624.5282.74.camel@surprise> In-Reply-To: <1383602624.5282.74.camel@surprise> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg00299.txt.bz2 On 11/04/2013 05:03 PM, David Malcolm wrote: > On Mon, 2013-11-04 at 16:43 -0500, David Malcolm wrote: >> On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote: >>> On 11/01/2013 06:58 PM, David Malcolm wrote: >>>> On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote: >>>>> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote: >>>>>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote: >>>>>>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote: >>>>>>>> static inline void >>>>>>>> ! gimple_call_set_lhs (gimple gs, tree lhs) >>>>>>>> { >>>>>>>> - GIMPLE_CHECK (gs, GIMPLE_CALL); >>>>> The checking you are removing here. >>>>> >>>>>> What checking? There ought to be no checking at all in this >>>>>> example... gimple_build_call_vec returns a gimple_call, and >>>>>> gimple_call_set_lhs() doesn't have to check anything because it >>>>>> only accepts gimple_call's.. so there is no checking other than the >>>>>> usual "does my parameter match" that the compiler has to do... >>>>> and want to replace it by checking of the types at compile time. >>>>> The problem is that it uglifies the source too much, and, when you >>>>> actually don't have a gimple_call but supposedly a base class of it, >>>>> I expect you'd do as_a which is not only further uglification, but has >>>>> runtime cost also for --enable-checking=release. >>>> I can have a look next week at every call to gimple_call_set_lhs in the >>>> tree, and see to what extent we know at compile-time that the initial >>>> arg is indeed a call (of the ones I quickly grepped just now, most are >>>> from gimple_build_call and friends, but one was from a gimple_copy). >>>> >>>> FWIW I did some performance testing of the is_a/as_a code in the earlier >>>> version of the patch, and it didn't have a noticable runtime cost >>>> compared to the GIMPLE_CHECK in the existing code: >>>> Size of compiler executable: >>>> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html >>>> Compile times: >>>> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html >>> I actually really dislike as_a<> and is_a<>, and think code needs to be >>> restructured rather than use them, other than possibly at the very >>> bottom level when we're allocating memory or something like that, or >>> some kind of emergency :-)... If we require frequent uses of those, >>> I'd be against it, I find them quite ugly. >>> >>> Like I said in the other reply, no rush, I don't think any of this >>> follow up is appropriate this late in stage 1. > I wasn't aware that there was a ramp in conservatism within stage1 - I > thought that we had a "large (tested) changes are OK" attitude > throughout all of stage1, and that the switch to conservatism only began > at the transition to stage3. (and have been frantically attempting to > get my big changes in before November 21st - should I be rethinking > this?) You read a lot into things :-) No, what I mean its too late to get a full change in, and partial changes don't seem worthwhile to me.. ie, I don't think there is much point in partially converting one or two gimple stmt kinds and leaving the others unconverted. so converting gimple_call_stmt and some of its access methods doesnt seem worthwhile to me right now when we could do the entire thing during the next stage1 for instance. There isnt enough benefit. Especially since as the work progresses you may discover improvements that make you go back and unwind some of the changes you would now commit. I believe this is a large enough body of work that needs some serious implementation before any of it would be appropriate for mainline. That said, the patch which enables this is more self contained, so wouldn't be subject to that. Its a matter of whether it has enough merit of its own to go in. Having the first patch in mainline would actually allow someone to experiment more easily during the "off season" if they wanted to, but wouldn't be mandatory since they could apply it to their own branch to work on. Andrew