This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign


On Fri, Nov 07, 2014 at 10:01:45PM +0100, Richard Biener wrote:
> > --- a/gcc/tree-ssa-tail-merge.c
> > +++ b/gcc/tree-ssa-tail-merge.c
> > @@ -484,7 +484,7 @@ same_succ_hash (const_same_succ e)
> >
> >        hstate.add_int (gimple_code (stmt));
> >        if (is_gimple_assign (stmt))
> > -       hstate.add_int (gimple_assign_rhs_code (stmt));
> > +       hstate.add_int (gimple_assign_rhs_code (as_a <gassign *> (stmt)));
> >        if (!is_gimple_call (stmt))
> >         continue;
> >        if (gimple_call_internal_p (stmt))
> > @@ -1172,8 +1172,10 @@ gimple_equal_p (same_succ same_succ, gimple s1, gimple s2)
> >        if (TREE_CODE (lhs1) != SSA_NAME
> >           && TREE_CODE (lhs2) != SSA_NAME)
> >         return (operand_equal_p (lhs1, lhs2, 0)
> > -               && gimple_operand_equal_value_p (gimple_assign_rhs1 (s1),
> > -                                                gimple_assign_rhs1 (s2)));
> > +               && gimple_operand_equal_value_p (gimple_assign_rhs1 (
> > +                                                  as_a <gassign *> (s1)),
> > +                                                gimple_assign_rhs1 (
> > +                                                  as_a <gassign *> (s2))));
> 
> Just a comment as these patches flow by - I think this is a huge step
> backwards from "enforcing" s1/s2 being a gimple_assign inside
> gimple_assign_rhs1 to this as_a <gassign *> boilerplate at _each_ callsite!
> 
> Which means this step of the refactoring is totally broken and probably
> requires much more manual work to avoid this kind of uglyness.
> 
> I definitely won't approve of this kind of changes.

I have to agree with this, this is too ugly to live with.
I must say I don't find anything wrong with what we have right now,
unlike RTL checking, the gimple checking is inexpensive, and much better
to do it that way then enforce all all developers to write it this way.
Otherwise we'll end up with code as ugly as in LLVM :(.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]