[PATCH] Fix PR52756
After going through a gazillion of candidate fixes for PR52756, a
case where jump threading destroys loops in a non-recoverable way,
I settled with the following. The issue is that we thread both
the loop latch and the loop entry edge but the code is not prepared
for that. Another possible fix would be to unconditionally throw
away threadings if we threaded the latch based on the fact that
the rest of the edges no longer are loop entry edges (but threading
them may create one, I think even still one that will end up
creating a multiple entry loop).
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Thanks,
Richard.
2012-04-02 Richard Guenther rguent...@suse.de
PR tree-optimization/52756
* tree-ssa-threadupdate.c (thread_through_loop_header): After
threading through the loop latch re-start the function to
double-check the rest of the threading opportunities.
* gcc.dg/torture/pr52756.c: New testcase.
Index: gcc/testsuite/gcc.dg/torture/pr52756.c
===
*** gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
***
*** 0
--- 1,9
+ /* { dg-do compile } */
+
+ void Env_FetchObj0AttrOffset (unsigned int NumFields, int *Status)
+ {
+ int Found = 0;
+ if (NumFields)
+ while ((*Status == 0) NumFields-- 0 Found == 0)
+ Found = 1;
+ }
Index: gcc/tree-ssa-threadupdate.c
===
*** gcc/tree-ssa-threadupdate.c (revision 186066)
--- gcc/tree-ssa-threadupdate.c (working copy)
*** thread_through_loop_header (struct loop
*** 838,843
--- 838,844
edge_iterator ei;
basic_block tgt_bb, atgt_bb;
enum bb_dom_status domst;
+ bool threaded_latch = false;
/* We have already threaded through headers to exits, so all the threading
requests now are to the inside of the loop. We need to avoid creating
*** thread_through_loop_header (struct loop
*** 908,913
--- 909,915
if (single_succ_p (header))
goto fail;
+ thread_rest:
if (latch-aux)
{
if (THREAD_TARGET2 (latch))
*** thread_through_loop_header (struct loop
*** 916,922
tgt_bb = tgt_edge-dest;
}
else if (!may_peel_loop_headers
! !redirection_block_p (loop-header))
goto fail;
else
{
--- 918,924
tgt_bb = tgt_edge-dest;
}
else if (!may_peel_loop_headers
! !redirection_block_p (header))
goto fail;
else
{
*** thread_through_loop_header (struct loop
*** 950,956
if (!tgt_bb)
{
/* There are no threading requests. */
! return false;
}
/* Redirecting to empty loop latch is useless. */
--- 952,958
if (!tgt_bb)
{
/* There are no threading requests. */
! return threaded_latch;
}
/* Redirecting to empty loop latch is useless. */
*** thread_through_loop_header (struct loop
*** 971,977
loop-header = NULL;
loop-latch = NULL;
loops_state_set (LOOPS_NEED_FIXUP);
! return thread_block (header, false);
}
if (tgt_bb-loop_father-header == tgt_bb)
--- 973,979
loop-header = NULL;
loop-latch = NULL;
loops_state_set (LOOPS_NEED_FIXUP);
! return threaded_latch | thread_block (header, false);
}
if (tgt_bb-loop_father-header == tgt_bb)
*** thread_through_loop_header (struct loop
*** 994,1002
loop-latch = thread_single_edge (latch);
gcc_assert (single_succ (loop-latch) == tgt_bb);
loop-header = tgt_bb;
/* Thread the remaining edges through the former header. */
! thread_block (header, false);
}
else
{
--- 996,1005
loop-latch = thread_single_edge (latch);
gcc_assert (single_succ (loop-latch) == tgt_bb);
loop-header = tgt_bb;
+ threaded_latch = true;
/* Thread the remaining edges through the former header. */
! goto thread_rest;
}
else
{
*** fail:
*** 1039,1045
free (e-aux);
e-aux = NULL;
}
! return false;
}
/* Walk through the registered jump threads and convert them into a
--- 1042,1048
free (e-aux);
e-aux = NULL;
}
! return threaded_latch;
}
/* Walk through the registered jump threads and convert them into a
Re: [PATCH] Fix PR52756
On Mon, 2 Apr 2012, Richard Guenther wrote:
After going through a gazillion of candidate fixes for PR52756, a
case where jump threading destroys loops in a non-recoverable way,
I settled with the following. The issue is that we thread both
the loop latch and the loop entry edge but the code is not prepared
for that. Another possible fix would be to unconditionally throw
away threadings if we threaded the latch based on the fact that
the rest of the edges no longer are loop entry edges (but threading
them may create one, I think even still one that will end up
creating a multiple entry loop).
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
And revealed one missed optimization, gcc.dg/tree-ssa/ssa-dom-thread-2.c
Thus the following is another try to fix things.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2012-04-02 Richard Guenther rguent...@suse.de
PR tree-optimization/52756
* tree-ssa-threadupdate.c (def_split_header_continue_p): New function.
(thread_through_loop_header): After threading through the loop latch
remove the split part from the loop and clear further threading
opportunities that would create a multiple entry loop.
* gcc.dg/torture/pr52756.c: New testcase.
Index: gcc/testsuite/gcc.dg/torture/pr52756.c
===
*** gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
***
*** 0
--- 1,9
+ /* { dg-do compile } */
+
+ void Env_FetchObj0AttrOffset (unsigned int NumFields, int *Status)
+ {
+ int Found = 0;
+ if (NumFields)
+ while ((*Status == 0) NumFields-- 0 Found == 0)
+ Found = 1;
+ }
Index: gcc/tree-ssa-threadupdate.c
===
*** gcc/tree-ssa-threadupdate.c (revision 186082)
--- gcc/tree-ssa-threadupdate.c (working copy)
*** determine_bb_domination_status (struct l
*** 826,831
--- 826,842
return (bb_reachable ? DOMST_DOMINATING : DOMST_LOOP_BROKEN);
}
+ /* Return true if BB is part of the new pre-header that is created
+when threading the latch to DATA. */
+
+ static bool
+ def_split_header_continue_p (const_basic_block bb, const void *data)
+ {
+ const_basic_block new_header = (const_basic_block) data;
+ return (bb-loop_father == new_header-loop_father
+ bb != new_header);
+ }
+
/* Thread jumps through the header of LOOP. Returns true if cfg changes.
If MAY_PEEL_LOOP_HEADERS is false, we avoid threading from entry edges
to the inside of the loop. */
*** thread_through_loop_header (struct loop
*** 990,1000
--- 1001,1046
if (latch-aux)
{
+ basic_block *bblocks;
+ unsigned nblocks, i;
+
/* First handle the case latch edge is redirected. */
loop-latch = thread_single_edge (latch);
gcc_assert (single_succ (loop-latch) == tgt_bb);
loop-header = tgt_bb;
+ /* Remove the new pre-header blocks from our loop. */
+ bblocks = XCNEWVEC (basic_block, loop-num_nodes);
+ nblocks = dfs_enumerate_from (header, 0, def_split_header_continue_p,
+ bblocks, loop-num_nodes, tgt_bb);
+ for (i = 0; i nblocks; i++)
+ {
+ remove_bb_from_loops (bblocks[i]);
+ add_bb_to_loop (bblocks[i], loop_outer (loop));
+ }
+ free (bblocks);
+
+ /* Cancel remaining threading requests that would make the
+loop a multiple entry loop. */
+ FOR_EACH_EDGE (e, ei, header-preds)
+ {
+ edge e2;
+ if (e-aux == NULL)
+ continue;
+
+ if (THREAD_TARGET2 (e))
+ e2 = THREAD_TARGET2 (e);
+ else
+ e2 = THREAD_TARGET (e);
+
+ if (e-src-loop_father != e2-dest-loop_father
+ e2-dest != loop-header)
+ {
+ free (e-aux);
+ e-aux = NULL;
+ }
+ }
+
/* Thread the remaining edges through the former header. */
thread_block (header, false);
}
Re: [PATCH] Fix PR52756
On Mon, Apr 2, 2012 at 5:32 AM, Richard Guenther rguent...@suse.de wrote:
After going through a gazillion of candidate fixes for PR52756, a
case where jump threading destroys loops in a non-recoverable way,
I settled with the following. The issue is that we thread both
the loop latch and the loop entry edge but the code is not prepared
for that. Another possible fix would be to unconditionally throw
away threadings if we threaded the latch based on the fact that
the rest of the edges no longer are loop entry edges (but threading
them may create one, I think even still one that will end up
creating a multiple entry loop).
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Thanks,
Richard.
2012-04-02 Richard Guenther rguent...@suse.de
PR tree-optimization/52756
* tree-ssa-threadupdate.c (thread_through_loop_header): After
threading through the loop latch re-start the function to
double-check the rest of the threading opportunities.
* gcc.dg/torture/pr52756.c: New testcase.
Index: gcc/testsuite/gcc.dg/torture/pr52756.c
===
*** gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
***
*** 0
--- 1,9
+ /* { dg-do compile } */
+
+ void Env_FetchObj0AttrOffset (unsigned int NumFields, int *Status)
+ {
+ int Found = 0;
+ if (NumFields)
+ while ((*Status == 0) NumFields-- 0 Found == 0)
+ Found = 1;
+ }
Is compiler allowed to optimize this function into an empty body?
Will it be a useful testcase then?
--
H.J.
Re: [PATCH] Fix PR52756
On Mon, 2 Apr 2012, H.J. Lu wrote:
On Mon, Apr 2, 2012 at 5:32 AM, Richard Guenther rguent...@suse.de wrote:
After going through a gazillion of candidate fixes for PR52756, a
case where jump threading destroys loops in a non-recoverable way,
I settled with the following. The issue is that we thread both
the loop latch and the loop entry edge but the code is not prepared
for that. Another possible fix would be to unconditionally throw
away threadings if we threaded the latch based on the fact that
the rest of the edges no longer are loop entry edges (but threading
them may create one, I think even still one that will end up
creating a multiple entry loop).
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Thanks,
Richard.
2012-04-02 Richard Guenther rguent...@suse.de
PR tree-optimization/52756
* tree-ssa-threadupdate.c (thread_through_loop_header): After
threading through the loop latch re-start the function to
double-check the rest of the threading opportunities.
* gcc.dg/torture/pr52756.c: New testcase.
Index: gcc/testsuite/gcc.dg/torture/pr52756.c
===
*** gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr52756.c (revision 0)
***
*** 0
--- 1,9
+ /* { dg-do compile } */
+
+ void Env_FetchObj0AttrOffset (unsigned int NumFields, int *Status)
+ {
+ int Found = 0;
+ if (NumFields)
+ while ((*Status == 0) NumFields-- 0 Found == 0)
+ Found = 1;
+ }
Is compiler allowed to optimize this function into an empty body?
Will it be a useful testcase then?
I don't think so as *Status may trap.
Richard.
