This is the mail archive of the gcc-patches@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: [tuples] Convert tail recursion elimination


On Feb 16, 2008 10:09 PM, Richard Guenther <richard.guenther@gmail.com> wrote:
> On Feb 16, 2008 10:03 PM, Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote:
> > Hello,
> >
> > this patch converts tail recursion elimination pass,
> >
> > Zdenek
> >
> >         * tree-tailcall.c: Tuplify.
> >         * gimplify.c (force_gimple_operand): Remove ATTRIBUTE_UNUSED.
> >         * gimple.c (gimple_assign_copy_p): Do not consider unary operations
> >         to be copies.
> >         (copy_or_nop_cast_stmt_rhs): New.
> >         * gimple.h (copy_or_nop_cast_stmt_rhs): Declare.
> >
>
> > + /* If STMT is a copy or no-op cast statement, returns its rhs operand.
> > +    Otherwise returns NULL.  */
> > +
> > + tree
> > + copy_or_nop_cast_stmt_rhs (gimple stmt)
> > + {
> > +   tree rhs;
> > +
> > +   if (gimple_code (stmt) != GIMPLE_ASSIGN)
> > +     return NULL_TREE;
> > +
> > +   rhs = gimple_assign_rhs1 (stmt);
> > +   if (gimple_assign_copy_p (stmt))
> > +     return rhs;
> > +   if (gimple_assign_cast_p (stmt))
> > +     {
> > +       tree lhs_type = TREE_TYPE (gimple_assign_lhs (stmt));
> > +       tree rhs_type = TREE_TYPE (TREE_TYPE (rhs));
> > +       return TYPE_MODE (lhs_type) == TYPE_MODE (rhs_type) ? rhs : NULL_TREE;
>
> I don't understand this -- why would we make this distinction for a function
> named copy_or_nop_cast_stmt_rhs?  The only relevant predicate for
> type compatibility would be useless_type_conversion_p, but then this
> would not be a gimple_assign_cast_p but just a gimple_assign_copy_p.
>
> So, this function needs much better documentation or otherwise it will
> be abused for the wrong thing (or is already).

Note that even the original code looks wrong.  A NOP_EXPR that doesn't change
the mode but the precision _is_ effectively producing code by
truncating/extending the
value.  The original code should use STRIP_USELESS_TYPE_CONVERSIONS
which should be a no-op there.  I realize it is probably there because
of conversions
between different pointer types, but this is not the only thing it
does - so this case
should probably be checked for explicitly instead.

Richard.


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