This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Operator Strength Reduction on trees
- From: jimbob at google dot com (Robert Kennedy)
- To: Steven Bosscher <stevenb dot gcc at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Robert Kennedy <jimbob at google dot com>
- Date: Fri, 12 Jan 2007 16:01:04 -0800
- Subject: Re: [PATCH] Operator Strength Reduction on trees
- References: <17830.49431.391910.490248@whippen.corp.google.com> <200701120032.05961.steven@gcc.gnu.org>
- Reply-to: Robert Kennedy <jimbob at google dot com>
> +/* Return true if STMT1 dominates STMT2. Only valid when bb_for_stmt
> + (stmt1) == bb_for_stmt (stmt2). */
> +
> +static bool
> +locally_dominates (tree stmt1, tree stmt2)
> +{
> + block_stmt_iterator bsi;
> +
> + bsi = bsi_for_stmt (stmt1);
> + while (!bsi_end_p (bsi))
> + {
> + if (bsi_stmt (bsi) == stmt2)
> + return true;
> + bsi_next (&bsi);
> + }
> + return false;
> +}
>
> What is this for? Why can't you just insert at the start of the
> basic block that both statements are in?
Because the RHS of the new definition being added will depend on the
two statements. It has to be after (i.e., dominated by) them, not
before.
So maybe you meant why not insert at the end of the block. One reason:
typically the new statement will be the only place that uses the
results of the two old statements, and we would rather shorten the two
live ranges and lengthen the one than do the other way around. Only if
both old stmt results have other live uses do we make things worse by
inserting as early as possible.
> +/* Return the last non-control statement in a basic block, or NULL if
> + there is none. */
> +
> +static tree
> +last_nonjump_stmt (basic_block block)
> +{
> + block_stmt_iterator last = bsi_last (block);
> + tree stmt;
> +
> + if (bsi_end_p (last))
> + return NULL_TREE;
> + stmt = bsi_stmt (last);
> +
> + while (stmt_ends_bb_p (stmt))
> + {
> + bsi_prev (&last);
> + if (bsi_end_p (last))
> + return NULL_TREE;
> + stmt = bsi_stmt (last);
> + }
> + return stmt; /* bsi_stmt (last); */
> +}
>
> Was there ever a statement that was not bsi_last, which did end the
> basic block? Seems to me that this means the block should have been
> split. In other words, I don't believe you need the while loop
> here.
Is the while loop expensive? I don't see how two ifs (one to check,
one to assert that the while loop isn't needed) would be better.
Thanks for your constructive comments and interesting questions (to
many of which I don't have answers yet).
-- Robert