Re: PR tree-optimization/54985
On 10/23/2012 03:43 PM, Jakub Jelinek wrote:
On Tue, Oct 23, 2012 at 03:34:46PM -0600, Jeff Law wrote:
I think it should be backported to 4.7, perhaps with a few days delay after the
trunk commit.
Do we even have debug statements after control flow statements?
They shouldn't be there, so if you just give up the same way for gsi_stmt
NULL as well as non-control stmt, it shouldn't make a difference.
So last_stmt might be just shorter to type and more commonly used, nothing
more.
Done.
Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
* tree-ssa-threadedge.c (cond_arg_set_in_bb): Use last stmt.
diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
index 6249e2f..ddaa7d1 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -584,7 +584,7 @@ cond_arg_set_in_bb (edge e, basic_block bb)
{
ssa_op_iter iter;
use_operand_p use_p;
- gimple last = gsi_stmt (gsi_last_bb (e->dest));
+ gimple last = last_stmt (e->dest);
/* E->dest does not have to end with a control transferring
instruction. This can occurr when we try to extend a jump
Re: PR tree-optimization/54985
On 10/23/2012 03:43 PM, Jakub Jelinek wrote: On Tue, Oct 23, 2012 at 03:34:46PM -0600, Jeff Law wrote: I think it should be backported to 4.7, perhaps with a few days delay after the trunk commit. Do we even have debug statements after control flow statements? They shouldn't be there, so if you just give up the same way for gsi_stmt NULL as well as non-control stmt, it shouldn't make a difference. So last_stmt might be just shorter to type and more commonly used, nothing more. From looking at the existing code and last_stmt; for the cases we care about, they ought to be equivalent. Using last_stmt seems marginally clearer. I'll go ahead and make that change after the usual bootstrap & test cycle. jeff
Re: PR tree-optimization/54985
The following trivial patch seems to fix it.
Index: tree-ssa-threadedge.c
===
--- tree-ssa-threadedge.c (revision 192749)
+++ tree-ssa-threadedge.c (working copy)
@@ -743,7 +743,7 @@
safe to thread this edge. */
if (e->flags & EDGE_DFS_BACK)
{
- if (cond_arg_set_in_bb (e, e->dest, 1))
+ if (cond_arg_set_in_bb (e, e->dest))
goto fail;
}
@@ -787,7 +787,7 @@
of threading without having to re-run DOM or VRP. */
if (dest
&& ((e->flags & EDGE_DFS_BACK) == 0
- || ! cond_arg_set_in_bb (taken_edge, e->dest, 2)))
+ || ! cond_arg_set_in_bb (taken_edge, e->dest)))
{
/* We don't want to thread back to a block we have already
visited. This may be overly conservative. */
@@ -846,7 +846,7 @@
do
{
if ((e->flags & EDGE_DFS_BACK) == 0
- || ! cond_arg_set_in_bb (e3, e->dest, 3))
+ || ! cond_arg_set_in_bb (e3, e->dest))
e2 = thread_around_empty_block (e3,
dummy_cond,
handle_dominating_asserts,
Sharad
On Tue, Oct 23, 2012 at 4:48 PM, Sharad Singhai wrote:
> The trunk seems to be broken at r192749 due to this patch.
>
> ../../srctrunk/gcc/tree-ssa-threadedge.c: In function ‘void
> thread_across_edge(gimple_statement_d*, edge_def*, bool,
> vec_t**, tree_node* (*)(gimple_statement_d*,
> gimple_statement_d*))’:
> ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
> arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
> basic_block_def*)’
> ../../srctrunk/gcc/tree-ssa-threadedge.c:746: error: at this point in file
> ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
> arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
> basic_block_def*)’
> ../../srctrunk/gcc/tree-ssa-threadedge.c:790: error: at this point in file
> ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many
> arguments to function ‘bool cond_arg_set_in_bb(edge_def*,
> basic_block_def*)’
> ../../srctrunk/gcc/tree-ssa-threadedge.c:849: error: at this point in file
> make: *** [tree-ssa-threadedge.o] Error 1
>
> Sharad
Re: PR tree-optimization/54985
The trunk seems to be broken at r192749 due to this patch. ../../srctrunk/gcc/tree-ssa-threadedge.c: In function ‘void thread_across_edge(gimple_statement_d*, edge_def*, bool, vec_t**, tree_node* (*)(gimple_statement_d*, gimple_statement_d*))’: ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many arguments to function ‘bool cond_arg_set_in_bb(edge_def*, basic_block_def*)’ ../../srctrunk/gcc/tree-ssa-threadedge.c:746: error: at this point in file ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many arguments to function ‘bool cond_arg_set_in_bb(edge_def*, basic_block_def*)’ ../../srctrunk/gcc/tree-ssa-threadedge.c:790: error: at this point in file ../../srctrunk/gcc/tree-ssa-threadedge.c:583: error: too many arguments to function ‘bool cond_arg_set_in_bb(edge_def*, basic_block_def*)’ ../../srctrunk/gcc/tree-ssa-threadedge.c:849: error: at this point in file make: *** [tree-ssa-threadedge.o] Error 1 Sharad
Re: PR tree-optimization/54985
On Tue, Oct 23, 2012 at 03:34:46PM -0600, Jeff Law wrote: > >I think it should be backported to 4.7, perhaps with a few days delay after > >the > >trunk commit. > Do we even have debug statements after control flow statements? They shouldn't be there, so if you just give up the same way for gsi_stmt NULL as well as non-control stmt, it shouldn't make a difference. So last_stmt might be just shorter to type and more commonly used, nothing more. Jakub
Re: PR tree-optimization/54985
On 10/23/2012 03:22 PM, Jakub Jelinek wrote:
On Tue, Oct 23, 2012 at 03:21:59PM -0600, Jeff Law wrote:
On 10/23/2012 02:50 PM, Jakub Jelinek wrote:
+static bool
+cond_arg_set_in_bb (edge e, basic_block bb, int n)
+{
+ ssa_op_iter iter;
+ use_operand_p use_p;
+ gimple last = gsi_stmt (gsi_last_bb (e->dest));
Use gimple last = last_stmt (e->dest); instead? That way any possible
debug stmts are ignored.
I thought I'd already dealt with this before. I'll double-check and
take appropriate action.
Any opinions about pulling it into 4.7.x as that release is affected
by this codegen bug? I've got no strong opinions and I'm willing to
pull it onto the branch if you want it.
I think it should be backported to 4.7, perhaps with a few days delay after the
trunk commit.
Do we even have debug statements after control flow statements?
jeff
Re: PR tree-optimization/54985
On Tue, Oct 23, 2012 at 03:21:59PM -0600, Jeff Law wrote:
> On 10/23/2012 02:50 PM, Jakub Jelinek wrote:
> >>+static bool
> >>+cond_arg_set_in_bb (edge e, basic_block bb, int n)
> >>+{
> >>+ ssa_op_iter iter;
> >>+ use_operand_p use_p;
> >>+ gimple last = gsi_stmt (gsi_last_bb (e->dest));
> >
> >Use gimple last = last_stmt (e->dest); instead? That way any possible
> >debug stmts are ignored.
> I thought I'd already dealt with this before. I'll double-check and
> take appropriate action.
>
> Any opinions about pulling it into 4.7.x as that release is affected
> by this codegen bug? I've got no strong opinions and I'm willing to
> pull it onto the branch if you want it.
I think it should be backported to 4.7, perhaps with a few days delay after the
trunk commit.
Jakub
Re: PR tree-optimization/54985
On 10/23/2012 02:50 PM, Jakub Jelinek wrote:
On Tue, Oct 23, 2012 at 02:35:24PM -0600, Jeff Law wrote:
+/* Return TRUE if the statement at the end of e->dest depends on
+ the output of any statement in BB. Otherwise return FALSE.
+
+ This is used when we are threading a backedge and need to ensure
+ that temporary equivalences from BB do not affect the condition
+ in e->dest. */
+
+static bool
+cond_arg_set_in_bb (edge e, basic_block bb, int n)
+{
+ ssa_op_iter iter;
+ use_operand_p use_p;
+ gimple last = gsi_stmt (gsi_last_bb (e->dest));
Use gimple last = last_stmt (e->dest); instead? That way any possible
debug stmts are ignored.
I thought I'd already dealt with this before. I'll double-check and
take appropriate action.
Any opinions about pulling it into 4.7.x as that release is affected by
this codegen bug? I've got no strong opinions and I'm willing to pull
it onto the branch if you want it.
jeff
Re: PR tree-optimization/54985
On Tue, Oct 23, 2012 at 02:35:24PM -0600, Jeff Law wrote:
> +/* Return TRUE if the statement at the end of e->dest depends on
> + the output of any statement in BB. Otherwise return FALSE.
> +
> + This is used when we are threading a backedge and need to ensure
> + that temporary equivalences from BB do not affect the condition
> + in e->dest. */
> +
> +static bool
> +cond_arg_set_in_bb (edge e, basic_block bb, int n)
> +{
> + ssa_op_iter iter;
> + use_operand_p use_p;
> + gimple last = gsi_stmt (gsi_last_bb (e->dest));
Use gimple last = last_stmt (e->dest); instead? That way any possible
debug stmts are ignored.
Jakub
