From patchwork Tue Nov 5 17:19:07 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 288589 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)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 9F5F62C007E for ; Wed, 6 Nov 2013 04:19:57 +1100 (EST) 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; q=dns; s=default; b=wtoO0oylWa1umoK1 I631gs1XJzXSVg+CJQXPqgkbKnvdgMy384VmzN4Lo2ATCQqOq9YZaBqfYf/t6c4Y vcostzGvWUr5HshbamCI10ho5CBIHlyb+wURnE1RHaW7ms6BzNeztFuT0cBDPQfy Ay1g0IAyYbm5B0wkqwCsx5fmTkI= 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; s=default; bh=MFYAFjOR+RFVcrGWpMJpAK U63Aw=; b=dCaWHlfDmu5A/eJVKbYJFk4+eT+uxpAaSQaNPL2Zmjz65Gk98D+jZH Nn+VvgrpS5yYcjyrezHMF2pyb6WVccxcpxOdohw0y1x9bZLEfj4wOG+QU7Z7ytb1 Hj70gnazQlWroVa0sKlkUXq7htiQdpRbctGoP7asAsXJbhrpXnBaY= Received: (qmail 14688 invoked by alias); 5 Nov 2013 17:19:46 -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 14677 invoked by uid 89); 5 Nov 2013 17:19:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 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; Tue, 05 Nov 2013 17:19:42 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rA5HJYpu013992 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 5 Nov 2013 12:19:34 -0500 Received: from [10.18.25.13] ([10.18.25.13]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rA5HJXvv000482; Tue, 5 Nov 2013 12:19:34 -0500 Message-ID: <1383671947.5282.93.camel@surprise> Subject: [PATCH] Add gimple subclasses for every gimple code (was Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3)) From: David Malcolm To: Andrew MacLeod Cc: gcc-patches@gcc.gnu.org Date: Tue, 05 Nov 2013 12:19:07 -0500 In-Reply-To: <52741EE2.3030100@redhat.com> References: <5271CBF9.2070005@redhat.com> <1383236801-13234-1-git-send-email-dmalcolm@redhat.com> <52741EE2.3030100@redhat.com> Mime-Version: 1.0 X-IsSubscribed: yes On Fri, 2013-11-01 at 17:36 -0400, Andrew MacLeod wrote: > On 10/31/2013 12:26 PM, David Malcolm wrote: > > [Shamelessly hijacking Andrew's thread about gimple.h refactoring, > > since this seems on-topic for that, and I'm keen to hear from Andrew on > > how the following would interact with his work - I *think* our two > > cleanups are orthogonal. > > Mostly orthogonal anyway... just stomping on the same bits :-). > > Since you hijacked a planning thread, do you plan to take this any > further, or make this change and move on to something else? > > It is a start, but it doesnt do the rest of the work that needs doing to > really take advantage of it... which will be extensive. > for instance, we should change: > > static inline void > ! gimple_call_set_lhs (gimple gs, tree lhs) > { > - GIMPLE_CHECK (gs, GIMPLE_CALL); > gimple_set_op (gs, 0, lhs); > to > static inline void > ! gimple_call_set_lhs (gimple_statement_call *gs, tree lhs) > { > gimple_set_op (gs, 0, lhs); > > > but then every location that calls it needs an appropriate change: > > ! gimple call; > ! call = gimple_build_call_vec (build_fold_addr_expr_loc (0, > alias), vargs); > gimple_call_set_lhs (call, atree); > > --- 1518,1524 ---- > > ! gimple_statement_call *call; > ! call = as_a (gimple_build_call_vec > (build_fold_addr_expr_loc (0, alias), vargs)); > gimple_call_set_lhs (call, atree); > > And in fact there is a ripple effect to then change > gimple_build_call_vec to simply return a gimple_statement_call *... Then > this doesn't look as ugly either... > > ! gimple_statement_call *call; > ! call = gimple_build_call_vec (build_fold_addr_expr_loc (0, > alias), vargs); > gimple_call_set_lhs (call, atree); > > that is looking much better :-) > > > Leaving the names as they are should be ok, but the I'd also add a > typedef for the pointer without the 'statement' in the name.. ie > typedef gimple_statement_call *gimple_call; > That seems in line with what we do with 'gimple' right now and the short > form is the type we'd normally use. > > That adds a touch of difficulty with "as_a", since that requires the > type name, not the shorthand pointer.... so you have something like > gimple_call call = as_a blah(). > I think as the changes to use the gimple_call type are pushed through > all the callers and callee's, the requirement of as_a and the long name > being ugly begins to rapidly disappear... it'll only exist in the core > creation routines and no one will usually see it. So I don't *think* > this is an issue... but it is an ugly transition if its only partially > done. > > And eventually we can pull the accessor routines and others into the > class itself: > gimple_call call; > call = gimple_build_call_vec (build_fold_addr_expr_loc (0, > alias), vargs); > call->set_lhs (atree); > > Which results in a similar feel to the new gimple_type, gimple_decl, > etc. classes with my upcoming tree wrappers. Changing gimple statements > to behave something like this is actually mentioned in the plan, but out > in the future once trees have been taken care of. > > I would also plan to create instances for each of the gimple statements > that don't have them now, like gimple_statement_assign. Its lumped in as > a general GSS_WITH_MEM_OPS, so there is no gimple_statement_assign > class/struct to use. > > It would really be nice to use the DEFGSCODE macro and gimple.def to > make this happen automagically somehow... Its tantalizingly close now I > think, especially combined with the info in gsstruct.def... Although if > we are moving to proper classes eventually its probably better to > explicitly write the required class for a statement kind. > > That all said, this change enables that work to proceed if someone wants > to do it. > > My question is: Is anyone going to do it, and if so, who and when? :-) Here's a followup patch which ensures that every gimple code has its own subclass, by adding empty subclasses derived from the GSS_-based subclasses as appropriate (I don't bother for gimple codes that already have their own subclass due to having their own GSS layout). I also copied the comments from gimple.def into gimple.h, so that Doxygen picks up on the descriptions and uses them to describe each subclass. Posting for discussion (i.e. am still bootstrapping this; it also needs the gengtype fix posted as http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00425.html) You can see what the resulting gimple class hierarchy (as reported by Doxygen) looks like here: http://dmalcolm.fedorapeople.org/gcc/2013-11-05/doxygen/html/structgimple__statement__base.html commit ec6a05ed9ff15b4fc458c668cfd5227a1506042b Author: David Malcolm Date: Tue Nov 5 04:30:15 2013 -0500 Ensure every gimple code has a subclasses; document. Add empty subclasses to gimple.h as appropriate to ensure that every gimple code has its own gimple subclass. Add a copy of the documentation from gimple.def to every gimple subclass in gimple.h (both new and existing ones) so that the per-code documentation shows up per-class within Doxygen-generated docs. gcc/ * gimple.h (struct gimple_statement_error_mark): New subclass of gimple_statement_base. (struct gimple_statement_cond): New subclass of gimple_statement_with_ops. (struct gimple_statement_debug): New subclass of gimple_statement_with_ops. (gimple_statement_goto): New subclass of gimple_statement_with_ops. (gimple_statement_label): New subclass of gimple_statement_with_ops. (gimple_statement_switch): New subclass of gimple_statement_with_ops. (gimple_statement_assign): New subclass of gimple_statement_with_memory_ops. (gimple_statement_call): Add documentation from gimple.def. (gimple_statement_bind): Likewise. (gimple_statement_catch): Likewise. (gimple_statement_eh_filter): Likewise. (gimple_statement_eh_else): Likewise. (gimple_statement_eh_mnt): Likewise. (gimple_statement_phi): Likewise. (gimple_statement_resx): New subclass of gimple_statement_eh_ctrl. (gimple_statement_dispatch): Likewise. (gimple_statement_try): Add documentation from gimple.def. (gimple_statement_nop): New subclass of gimple_statement_base. (gimple_statement_wce): Add documentation from gimple.def. (gimple_statement_asm): Likewise. (gimple_statement_omp_critical): Likewise. (gimple_statement_omp_for): Likewise. (gimple_statement_omp_master): New subclass of gimple_statement_omp. (gimple_statement_omp_taskgroup): Likewise. (gimple_statement_omp_ordered): Likewise. (gimple_statement_omp_parallel): Add documentation from gimple.def. (gimple_statement_omp_task): Likewise. (gimple_statement_omp_section): New subclass of gimple_statement_omp. (gimple_statement_omp_sections): Add documentation from gimple.def. (gimple_statement_omp_sections_switch): New subclass of gimple_statement_base. (gimple_statement_omp_continue): Add documentation from gimple.def. (gimple_statement_omp_target): New subclass of gimple_statement_omp_parallel. (gimple_statement_omp_teams): New subclass of gimple_statement_omp_single. (gimple_predict): New subclass of gimple_statement_base. (gimple_statement_omp_atomic_load): Add documentation from gimple.def. (gimple_statement_omp_atomic_store): Update comment. (gimple_statement_omp_return): New subclass of gimple_statement_omp_atomic_store. (gimple_statement_transaction): Add documentation from gimple.def. (gimple_statement_return): New subclass of gimple_statement_with_memory_ops. diff --git a/gcc/gimple.h b/gcc/gimple.h index 35bfa06..318953c 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -223,6 +223,13 @@ struct GTY((desc ("gimple_statement_structure (&%h)"), tag ("GSS_BASE"), gimple GTY((skip)) prev; }; +/* code == GIMPLE_ERROR_MARK, an error marker. */ + +struct GTY(()) + gimple_statement_error_mark : public gimple_statement_base +{ + /* no additional fields; this uses the layout for GSS_BASE. */ +}; /* Base structure for tuples with operands. */ @@ -255,6 +262,81 @@ struct GTY((tag("GSS_WITH_OPS"))) tree GTY((length ("%h.num_ops"))) op[1]; }; +/* code == GIMPLE_COND: + + GIMPLE_COND + represents the conditional jump: + + if (OP1 COND_CODE OP2) goto TRUE_LABEL else goto FALSE_LABEL + + COND_CODE is the tree code used as the comparison predicate. It + must be of class tcc_comparison. + + OP1 and OP2 are the operands used in the comparison. They must be + accepted by is_gimple_operand. + + TRUE_LABEL and FALSE_LABEL are the LABEL_DECL nodes used as the + jump target for the comparison. */ + +struct GTY(()) + gimple_statement_cond : public gimple_statement_with_ops +{ + /* no additional fields; this uses the layout for GSS_WITH_OPS. */ +}; + +/* code == GIMPLE_DEBUG, a debug statement. */ + +struct GTY(()) + gimple_statement_debug : public gimple_statement_with_ops +{ + /* no additional fields; this uses the layout for GSS_WITH_OPS. */ +}; + +/* code == GIMPLE_GOTO + + GIMPLE_GOTO represents unconditional jumps. + TARGET is a LABEL_DECL or an expression node for computed GOTOs. */ + +struct GTY(()) + gimple_statement_goto : public gimple_statement_with_ops +{ + /* no additional fields; this uses the layout for GSS_WITH_OPS. */ +}; + +/* code == GIMPLE_LABEL + + GIMPLE_LABEL