This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: David Malcolm <dmalcolm at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, GCC Development <gcc at gcc dot gnu dot org>, Jeff Law <law at redhat dot com>
- Date: Fri, 7 Nov 2014 22:23:28 +0100
- Subject: Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
- Authentication-results: sourceware.org; auth=none
- References: <1415373690-26193-1-git-send-email-dmalcolm at redhat dot com> <1415373690-26193-5-git-send-email-dmalcolm at redhat dot com> <CAFiYyc2rg+T5d7rkTBPLeWKGnWMgv07j_N8KaFfr0NK1CjsfVQ at mail dot gmail dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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