[PATCH] Add gimple subclasses for every gimple code (was Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3))

[Michael Matz]

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.