On Fri, 2013-12-13 at 12:13 -0500, Andrew MacLeod wrote:
> On 12/13/2013 10:58 AM, David Malcolm wrote:
> > {
> > gimple stmt = SSA_NAME_DEF_STMT (x);
> > @@ -2162,7 +2162,7 @@ chain_of_csts_start (struct loop *loop, tree x)
> > if (gimple_code (stmt) == GIMPLE_PHI)
> > {
> > if (bb == loop->header)
> > - return stmt;
> > + return stmt->as_a_gimple_phi ();
> >
> > return NULL;
> > }
> > @@ -2195,10 +2195,10 @@ chain_of_csts_start (struct loop *loop, tree x)
> >
> > If such phi node exists, it is returned, otherwise NULL is returned.
> > */
> >
>
> I dislike separating the checking of gimple_code () and the following
> as_a. I rather envisioned this sort of thing as being more of an
> abstraction improvement if we never have to check gimple_code()...
> Then you are also less locked into a specific implementation.
>
> So something more like:
>
> if (gimple_phi phi = stmt->dyncast_gimple_phi ())
> {
> if (bb == loop->header)
> return phi;
> }
>
>
> IMO anyway...
Thanks.
My goal is to use these stronger types (a) to move type-checking to
compile-time and (b) to (i hope) improve the readability of the code.
I'm not trying to switch away from gimple_code for the home-grown RTTI
per se.
However, given that you prefer the above style, I'm now opting to use
dyn_cast for the above kind of test in my ongoing work on this.
The other consideration is that I'm trying to minimize the invasiveness
of the patches, to avoid the amount of conflicts that will occur when
trying to merge this (for next stage1). So I'm sometimes tactically
avoiding some constructs, e.g. to avoid needing to reindent large
suites.
FWIW I'm currently at 90 patches, and have reached some kind of halfway
point, with 162 gimple_foo_ access functions now taking a more concrete
type that "gimple" [1]; 159 to go. That said, I think these accessors
are something of a surface detail - I'm more interested in such
"concretizing" of types *throughout* the middle-end, rather than just
focusing on the gimple_foo_ access functions; for example, I now have
the callgraph edge statements being "gimple_call" rather than just
"gimple". It's the latter kind of deeper change to typesafety that I'm
most excited about it.
Andrew: hopefully this is all compatible with your proposed changes to
types and expressions? I'm trying to just touch the statements
themselves.
Dave
[1] including all of gimple_asm_*, gimple_bind_*, gimple_catch_*,
gimple_eh_dispatch_*, gimple_eh_else_*, gimple_omp_atomic_load_*,
gimple_omp_atomic_store_*, gimple_omp_continue_*, gimple_resx_*,
gimple_switch_*, gimple_transaction_*.
