From patchwork Wed Nov 19 22:28:07 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 412534 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 0AAA414010F for ; Thu, 20 Nov 2014 09:32:49 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version:content-transfer-encoding; q=dns; s= default; b=DY8eNp8uB565Tg8wrmbfsUoHLFF3OxI90BzssQzVWJw6kAM1hHE0d yxWStiScgo/Jrf3g4Z7/3L6rGEU/jbmTJDN8jerQzES9wRVnC0/pCGHk6Td1Xfrp 8q2IjUmnjohhMizrBo+mKCvJ+Tlu8q4sPZBQMdE++0ho6ftb6IAwj0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version:content-transfer-encoding; s=default; bh=z1i04x5JUHKMuEzwTndm3uLdflg=; b=QF3HpjoAqvBb107vWs6havi6czto WVoPOrKbtHRuNF3M2LI46oLnVk5cm1E3H9/ymlWLHSZ7gB5YhTNtNm8EVzjLWpKO Qmq99EGhAI72C8oMyfuCoUtE+mXbVvGDvmILcFSZFM0tO4VRl6fGAhmxdyG07EdF KZfEB3/la6d+bgk= Received: (qmail 26690 invoked by alias); 19 Nov 2014 22:32:40 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 26598 invoked by uid 89); 19 Nov 2014 22:32:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 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; Wed, 19 Nov 2014 22:32:37 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAJMWasw013997 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 19 Nov 2014 17:32:36 -0500 Received: from [10.3.237.203] (vpn-237-203.phx2.redhat.com [10.3.237.203]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAJMWZU8023241; Wed, 19 Nov 2014 17:32:36 -0500 Message-ID: <1416436087.3562.81.camel@surprise> Subject: Re: "gimple-classes-v2-option-3" git branch committed to svn trunk as r217787 From: David Malcolm To: Richard Biener Cc: Andrew MacLeod , gcc-patches@gcc.gnu.org Date: Wed, 19 Nov 2014 17:28:07 -0500 In-Reply-To: <1416435844.3562.78.camel@surprise> References: <1416420732.3562.49.camel@surprise> <546CEEDF.10007@redhat.com> <8C1B6C93-EAA6-4F10-84BB-743499104758@gmail.com> <546D0724.3080006@redhat.com> <1416435844.3562.78.camel@surprise> Mime-Version: 1.0 X-IsSubscribed: yes On Wed, 2014-11-19 at 17:24 -0500, David Malcolm wrote: > On Wed, 2014-11-19 at 22:36 +0100, Richard Biener wrote: > > On November 19, 2014 10:09:56 PM CET, Andrew MacLeod wrote: > > >On 11/19/2014 03:43 PM, Richard Biener wrote: > > >> On November 19, 2014 8:26:23 PM CET, Andrew MacLeod > > > wrote: > > >>> On 11/19/2014 01:12 PM, David Malcolm wrote: > > >>> > > >>>> (A) could become: > > >>>> > > >>>> greturn *stmt = gsi->as_a_greturn (); > > >>>> > > >>>> (B) could become: > > >>>> > > >>>> stmt = gsi->dyn_cast (); > > >>>> if (!stmt) > > >>>> or: > > >>>> > > >>>> stmt = gsi->dyn_cast_gcall (); > > >>>> if (!stmt) > > >>>> > > >>>> or maybe: > > >>>> > > >>>> stmt = gsi->is_a_gcall (); > > >>>> if (!stmt) > > >>>> > > >>>> An earlier version of my patches had casting methods within the > > >>>> gimple_statement_base class, which were rejected; the above > > >proposals > > >>>> would instead put them within gimple_stmt_iterator. > > >>>> > > >>> I would like all gsi routines to be consistent, not a mix of > > >functions > > >>> and methods. > > >>> so something like > > >>> > > >>> stmt = gsi_as_call (gsi); > > >>> stmt = gsi_dyn_call (gsi); > > >>> > > >>> or we need to change gsi_stmt() and friends into methods and access > > >>> them > > >>> as gsi->stmt ().. which is possibly better, but that much more > > >>> intrusive again (another 2000+ locations). > > >>> If we switched to methods everywhere for gsi, I'd prefer something > > >like > > >>> gsi->as_a_call () > > >>> gsi->is_a_call () > > >>> gsi->dyn_cast_call () > > >>> > > >>> I think its more readable... and it removes a dependency on the > > >>> implementation.. so if we ever decide to change the name of 'gcall' > > >>> down > > >>> the road to using a namespace, and make it gimple::call or whatever, > > >we > > >>> > > >>> wont have to change every single gsi-> location which has a > > >templated > > >>> use of the type. > > >>> > > >>> I'm also think this sort of thing could probably wait until next > > >stage > > >>> 1.. > > >>> > > >>> my 2 cents... > > >> Why not as_a (*gsi)? It would > > >> Add operator* to gsi of course. > > >> > > >> Richard. > > >> > > >> > > > > > >I could live with that form too. > > > > > >we often have an instance of gimple_stmt_iterator rather than a pointer > > > > > >to it, so wouldn't "operator gimple *()" to implicitly call gsi_stmt() > > > > > >when needed work better? (or "operator gimple ()" before the next > > >change) .. > > > > Not sure. The * matches how iterators work in STL... > > > > Note that for the cases where we pass a pointer to an iterator we can change those to use references to avoid writing **gsi. > > > > Richard. > > > > >Andrew > > I had a go at adding an operator * to gimple_stmt_iterator, using it > everywhere that we do an as_a<> or dyn_cast<> on the result of a > gsi_stmt, to abbreviate the gsi_stmt call down to one character. > > Patch attached; only lightly smoketested; am posting it for the sake of > discussion. > > I don't think this API will make the non-C++-fans happier; I think the > objection to the work I just merged is that it's adding more C++ than > those people are comfortable with. > > So although the attached patch makes things shorter (good), it's taking > things in a "more C++" direction (questionable). I'd hoped to paper > over the C++ somewhat. > > I suspect that any API which requires the of < > characters within the > implementation of a gimple pass to mean a template is going to give > those less happy with C++ a visceral "ugh" reaction. I wonder if > there's a way to spell these things that's concise and which doesn't > involve <> ? Answering my own question, user-defined conversion operators, though should they as_a or dyn_cast? Here's an example where they mean "as_a", radically shortening the conversion (shorter, in fact, that the pre-merger code): (again, I only checked that it compiles) Dave diff --git a/gcc/asan.c b/gcc/asan.c index be28ede..890e58b 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1902,7 +1902,7 @@ instrument_builtin_call (gimple_stmt_iterator *iter) return false; bool iter_advanced_p = false; - gcall *call = as_a (gsi_stmt (*iter)); + gcall *call = *iter; gcc_checking_assert (gimple_call_builtin_p (call, BUILT_IN_NORMAL)); diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h index fb6cc07..52106fa 100644 --- a/gcc/gimple-iterator.h +++ b/gcc/gimple-iterator.h @@ -33,6 +33,8 @@ struct gimple_stmt_iterator block/sequence is removed. */ gimple_seq *seq; basic_block bb; + + operator gcall * (); }; /* Iterator over GIMPLE_PHI statements. */ @@ -331,4 +333,11 @@ gsi_seq (gimple_stmt_iterator i) return *i.seq; } +inline +gimple_stmt_iterator::operator gcall * () +{ + return as_a (gsi_stmt (*this)); +} + + #endif /* GCC_GIMPLE_ITERATOR_H */