public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix loop-ch
@ 2023-04-21 13:36 Jan Hubicka
  2023-04-21 15:40 ` Jan Hubicka
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Hubicka @ 2023-04-21 13:36 UTC (permalink / raw)
  To: gcc-patches, kubanek0ondrej

Hi,
Ondrej Kubanek implemented profiling of loop histograms which sould be useful to improve
i.e. quality of loop peeling of verctorization.  However it turns out that most of histograms
are lost on the way from profiling to loop peeling pass (about 90%).  One common case is the
following latent bug in loop header copying which forgets to update the loop header pointer.

Curiously enough it does work to make single latch and preheader edge by splitting basic blocks
but it works with wrong edge.  As a consequence every loop where loop header was copied is
removed from loop tree and inserted again losing all metadata included.

Patch correctly updates the loop structure and also add verification
that the loop tree is OK after all transforms which is failing without
the patch.

Bootstrapped/regtested x86_64-linux, plan to insteall this as obvious.

diff --git a/gcc/tree-ssa-loop-ch.cc b/gcc/tree-ssa-loop-ch.cc
index 6b332719411..2c56b3b3c31 100644
--- a/gcc/tree-ssa-loop-ch.cc
+++ b/gcc/tree-ssa-loop-ch.cc
@@ -557,6 +557,17 @@ ch_base::copy_headers (function *fun)
 	    }
 	}
 
+      /* Update header of the loop.  */
+      loop->header = header;
+      /* Find correct latch.  We only duplicate chain of conditionals so
+         there should be precisely two edges to the new header.  One entry
+         edge and one to latch.  */
+      FOR_EACH_EDGE (e, ei, loop->header->preds)
+	if (header != e->src)
+	  {
+	    loop->latch = e->src;
+	    break;
+	  }
       /* Ensure that the latch and the preheader is simple (we know that they
 	 are not now, since there was the loop exit condition.  */
       split_edge (loop_preheader_edge (loop));
@@ -583,6 +594,8 @@ ch_base::copy_headers (function *fun)
 
   if (changed)
     {
+      if (flag_checking)
+	verify_loop_structure ();
       update_ssa (TODO_update_ssa);
       /* After updating SSA form perform CSE on the loop header
 	 copies.  This is esp. required for the pass before

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Fix loop-ch
  2023-04-21 13:36 Fix loop-ch Jan Hubicka
@ 2023-04-21 15:40 ` Jan Hubicka
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Hubicka @ 2023-04-21 15:40 UTC (permalink / raw)
  To: gcc-patches, kubanek0ondrej

> Hi,
> Ondrej Kubanek implemented profiling of loop histograms which sould be useful to improve
> i.e. quality of loop peeling of verctorization.  However it turns out that most of histograms
> are lost on the way from profiling to loop peeling pass (about 90%).  One common case is the
> following latent bug in loop header copying which forgets to update the loop header pointer.
> 
> Curiously enough it does work to make single latch and preheader edge by splitting basic blocks
> but it works with wrong edge.  As a consequence every loop where loop header was copied is
> removed from loop tree and inserted again losing all metadata included.
> 
> Patch correctly updates the loop structure and also add verification
> that the loop tree is OK after all transforms which is failing without
> the patch.
> 
> Bootstrapped/regtested x86_64-linux, plan to insteall this as obvious.
> 
Hi,
sadly I managed to mix up patch and its WIP version in previous commit.
This patch adds the missing edge iterator and also fixes a side case
where new loop header would have multiple latches.

Bootstrapping/regtesting of x86_64-linux in progress.  It was tested
previously and passed, so I hope it will again and commit it to unbreak
master then.

gcc/ChangeLog:

	* tree-ssa-loop-ch.cc (ch_base::copy_headers): Fix previous
	commit.

diff --git a/gcc/tree-ssa-loop-ch.cc b/gcc/tree-ssa-loop-ch.cc
index 560df39893e..9487e7f3e55 100644
--- a/gcc/tree-ssa-loop-ch.cc
+++ b/gcc/tree-ssa-loop-ch.cc
@@ -484,7 +484,10 @@ ch_base::copy_headers (function *fun)
       /* Ensure that the header will have just the latch as a predecessor
 	 inside the loop.  */
       if (!single_pred_p (exit->dest))
-	exit = single_pred_edge (split_edge (exit));
+	{
+	  header = split_edge (exit);
+	  exit = single_pred_edge (header);
+	}
 
       entry = loop_preheader_edge (loop);
 
@@ -547,16 +550,17 @@ ch_base::copy_headers (function *fun)
       /* Find correct latch.  We only duplicate chain of conditionals so
 	 there should be precisely two edges to the new header.  One entry
 	 edge and one to latch.  */
+      edge_iterator ei;
+      edge e;
       FOR_EACH_EDGE (e, ei, loop->header->preds)
 	if (header != e->src)
 	  {
 	    loop->latch = e->src;
 	    break;
 	  }
-      /* Ensure that the latch and the preheader is simple (we know that they
-	 are not now, since there was the loop exit condition.  */
-      split_edge (loop_preheader_edge (loop));
-      split_edge (loop_latch_edge (loop));
+      /* Ensure that the latch is simple.  */
+      if (!single_succ_p (loop_latch_edge (loop)->src))
+	split_edge (loop_latch_edge (loop));
 
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-04-21 15:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21 13:36 Fix loop-ch Jan Hubicka
2023-04-21 15:40 ` Jan Hubicka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).