From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17708 invoked by alias); 14 Nov 2013 13:04: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 17699 invoked by uid 89); 14 Nov 2013 13:04:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_50,RDNS_NONE autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from Unknown (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 14 Nov 2013 13:04:44 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E12A7A79B2; Thu, 14 Nov 2013 14:04:35 +0100 (CET) Date: Thu, 14 Nov 2013 15:06:00 -0000 From: Michael Matz To: Jeff Law Cc: David Malcolm , Andrew MacLeod , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Add gimple subclasses for every gimple code (was Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3)) In-Reply-To: <5284781F.3000604@redhat.com> Message-ID: References: <5271CBF9.2070005@redhat.com> <1383236801-13234-1-git-send-email-dmalcolm@redhat.com> <52741EE2.3030100@redhat.com> <1383671947.5282.93.camel@surprise> <1383800204.31927.33.camel@surprise> <527B25ED.9030804@redhat.com> <1383937364.31927.66.camel@surprise> <5284781F.3000604@redhat.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg01605.txt.bz2 Hi, On Thu, 14 Nov 2013, Jeff Law wrote: > Thanks. It's pretty much what I expected. Obviously for other codes > there may be a lot more changes that you have to slog through, but I > think this example shows the main concepts. > > Presumably in this new world order, the various gimple statement types > will continue to inherit from a base class. That seems somewhat > inevitable and implies a certain amount of downcasting (via whatever > means we agree upon). The worry, in my mind is how pervasive the > downcasting will be and how much of it we can get rid of over time. > > I may be wrong, but ISTM some of the downcasting is a result of not > providing certain capabilities via (pure?) virtual methods. For > example, expand_gimple_stmt_1 seems ripe for implementing as virtual > methods. Please no. A class with many methods hints at a bad design. If we were to add virtual methods for every piece of the hundred passes that have to do different things per instruction code we'd arrive at exactly that. Even though most of these methods would be more related to the pass they're in they would actually have to be implemented in the gimple class (and their inherited ones). I wouldn't want to see methods ala expand_me(), remove_me(), remove_me_for_predcom(), remove_me_for_some_other_pass(), optimize_me_for_{this,that,another}() and so on. > ISTM you could also have virtuals to build the statements, Virtual to build things? You mean constructors. > dump/pretty-print them, True. > verify them, Depends. Part of them can be verified without context information, and that would perhaps be sensible in some virtual methods. Part of the verification depends on context, other insns, the call graph, the CFG. Those should IMHO not be methods of gimple. > branch/edge redirection, That seems to me more related to the cfg, not to statements. Some statements need to be modified, true, but only very few subclasses (those that can transfer control). > estimations for inlining, Part of that (again the things you can determine with a statement alone, without much context, yes. > etc. ISTM that would eliminate a good chunk > of the downcasting. > > Just to be clear, I'm not asking you to make those changes, just for > your thoughts on approaches to eliminate the downcasting based on what > we've seen so far. Before adding a vtable pointer to gimple (or any other central, heavily used data structure) I'd go through great pain to avoid that. Ciao, Michael.