> On Sat, Oct 6, 2012 at 11:34 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
> > Hi,
> > I benchmarked the patch moving loop header copying and it is quite 
> > noticeable win.
> >
> > Some testsuite updating is needed. In many cases it is just because the
> > optimizations are now happening earlier.
> > There are however few testusite failures I have torubles to deal with:
> > ./testsuite/gcc/gcc.sum:FAIL: gcc.dg/tree-ssa/pr21559.c 
> > scan-tree-dump-times vrp1 "Threaded jump" 3
> > ./testsuite/gcc/gcc.sum:FAIL: gcc.dg/tree-ssa/ssa-dom-thread-2.c 
> > scan-tree-dump-times vrp1 "Jumps threaded: 1" 1
> > ./testsuite/gcc/gcc.sum:FAIL: gcc.dg/vect/O3-slp-reduc-10.c 
> > scan-tree-dump-times vect "vectorized 1 loops" 2
> > ./testsuite/g++/g++.sum:FAIL: g++.dg/tree-ssa/pr18178.C -std=gnu++98  
> > scan-tree-dump-times vrp1 "if " 1
> > ./testsuite/g++/g++.sum:FAIL: g++.dg/tree-ssa/pr18178.C -std=gnu++11  
> > scan-tree-dump-times vrp1 "if " 1
> >
> > This is mostly about VRP losing its ability to thread some jumps from the
> > duplicated loop header out of the loop across the loopback edge.  This 
> > seems to
> > be due to loop updating logic.  Do we care about these?
> 
> Yes, I think so.  At least we care that the optimized result is the same.

it is not, we really lose optimization in those testcases.
The ones that are still optimized well I updated in the patch bellow.
> 
> Can you elaborate on "due to loop updating logic"?

The problem is:
  /* We do not allow VRP information to be used for jump threading
     across a back edge in the CFG.  Otherwise it becomes too
     difficult to avoid eliminating loop exit tests.  Of course
     EDGE_DFS_BACK is not accurate at this time so we have to
     recompute it.  */
  mark_dfs_back_edges ();

  /* Do not thread across edges we are about to remove.  Just marking
     them as EDGE_DFS_BACK will do.  */
  FOR_EACH_VEC_ELT (edge, to_remove_edges, i, e)
    e->flags |= EDGE_DFS_BACK;

Loop header copying puts some conditional before loop and we want to thread
up to exit out of the loop (that I think it rather important optimization).
But it no longer happens before back edge is in the way.  At least that was
the case in the tree-ssa failures I analyzed.
> 
> Can you elaborate on the def_split_header_continue_p change?  Which probably
> should be tested and installed separately?

Yes, that one is latent bug.  The code is expecting that loop exit is recognized
by loop depth decreasing that is not true.
It reproduces as ICE during bootstrap with the patch.
I will regtest/bootstrap and commit it today.

Honza

Reply via email to