This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: David Malcolm <dmalcolm at redhat dot com>
- Cc: 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:01:45 +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>
On Fri, Nov 7, 2014 at 4:21 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> gcc/ChangeLog.gimple-classes:
> * tree-ssa-tail-merge.c (same_succ_hash): Add checked cast.
> (gimple_equal_p): Add checked casts.
> ---
> gcc/ChangeLog.gimple-classes | 5 +++++
> gcc/tree-ssa-tail-merge.c | 8 +++++---
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/ChangeLog.gimple-classes b/gcc/ChangeLog.gimple-classes
> index f43df63..0bd0421 100644
> --- a/gcc/ChangeLog.gimple-classes
> +++ b/gcc/ChangeLog.gimple-classes
> @@ -1,5 +1,10 @@
> 2014-11-06 David Malcolm <dmalcolm@redhat.com>
>
> + * tree-ssa-tail-merge.c (same_succ_hash): Add checked cast.
> + (gimple_equal_p): Add checked casts.
> +
> +2014-11-06 David Malcolm <dmalcolm@redhat.com>
> +
> * tree-ssa-structalias.c (find_func_aliases): Replace
> is_gimple_assign with a dyn_cast, introducing local gassign *
> "t_assign", using it in place of "t" for typesafety.
> diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
> index 5678657..b822214 100644
> --- 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.
Thanks,
Richard.
> else if (TREE_CODE (lhs1) == SSA_NAME
> && TREE_CODE (lhs2) == SSA_NAME)
> return vn_valueize (lhs1) == vn_valueize (lhs2);
> --
> 1.7.11.7
>