[1/4] vect: Set counts of early break exit blocks correctly [PR117790]

Message ID Z3u/rCEwydQh47qO@arm.com
State New
Headers
Series vect, cfgloopmanip: Fix profile consistency of early break loops [PR117790] |

Commit Message

Alex Coplan Jan. 6, 2025, 11:34 a.m. UTC
  This adds missing code to correctly set the counts of the exit blocks we
create when building the CFG for a vectorized early break loop.

Tested as a series on aarch64-linux-gnu, arm-linux-gnueabihf, and
x86_64-linux-gnu.  OK for trunk?

Thanks,
Alex

gcc/ChangeLog:

	PR tree-optimization/117790
	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
	Set profile counts for {main,alt}_loop_exit_block.
---
 gcc/tree-vect-loop-manip.cc | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Tamar Christina Jan. 15, 2025, 2:07 p.m. UTC | #1
Ping

> -----Original Message-----
> From: Alex Coplan <Alex.Coplan@arm.com>
> Sent: Monday, January 6, 2025 11:34 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Biener <rguenther@suse.de>; Jan Hubicka <hubicka@ucw.cz>; Tamar
> Christina <Tamar.Christina@arm.com>
> Subject: [PATCH 1/4] vect: Set counts of early break exit blocks correctly
> [PR117790]
> 
> This adds missing code to correctly set the counts of the exit blocks we
> create when building the CFG for a vectorized early break loop.
> 
> Tested as a series on aarch64-linux-gnu, arm-linux-gnueabihf, and
> x86_64-linux-gnu.  OK for trunk?
> 
> Thanks,
> Alex
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/117790
> 	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
> 	Set profile counts for {main,alt}_loop_exit_block.
> ---
>  gcc/tree-vect-loop-manip.cc | 10 ++++++++++
>  1 file changed, 10 insertions(+)
  
Tamar Christina Jan. 24, 2025, 9:17 a.m. UTC | #2
ping

> -----Original Message-----
> From: Tamar Christina
> Sent: Wednesday, January 15, 2025 2:07 PM
> To: Alex Coplan <Alex.Coplan@arm.com>; gcc-patches@gcc.gnu.org
> Cc: Richard Biener <rguenther@suse.de>; Jan Hubicka <hubicka@ucw.cz>
> Subject: RE: [PATCH 1/4] vect: Set counts of early break exit blocks correctly
> [PR117790]
> 
> Ping
> 
> > -----Original Message-----
> > From: Alex Coplan <Alex.Coplan@arm.com>
> > Sent: Monday, January 6, 2025 11:34 AM
> > To: gcc-patches@gcc.gnu.org
> > Cc: Richard Biener <rguenther@suse.de>; Jan Hubicka <hubicka@ucw.cz>; Tamar
> > Christina <Tamar.Christina@arm.com>
> > Subject: [PATCH 1/4] vect: Set counts of early break exit blocks correctly
> > [PR117790]
> >
> > This adds missing code to correctly set the counts of the exit blocks we
> > create when building the CFG for a vectorized early break loop.
> >
> > Tested as a series on aarch64-linux-gnu, arm-linux-gnueabihf, and
> > x86_64-linux-gnu.  OK for trunk?
> >
> > Thanks,
> > Alex
> >
> > gcc/ChangeLog:
> >
> > 	PR tree-optimization/117790
> > 	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
> > 	Set profile counts for {main,alt}_loop_exit_block.
> > ---
> >  gcc/tree-vect-loop-manip.cc | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
  
Tamar Christina Feb. 3, 2025, 2:45 p.m. UTC | #3
Ping

> -----Original Message-----
> From: Tamar Christina
> Sent: Friday, January 24, 2025 9:17 AM
> To: Alex Coplan <Alex.Coplan@arm.com>; 'gcc-patches@gcc.gnu.org' <gcc-
> patches@gcc.gnu.org>
> Cc: 'Richard Biener' <rguenther@suse.de>; 'Jan Hubicka' <hubicka@ucw.cz>
> Subject: RE: [PATCH 1/4] vect: Set counts of early break exit blocks correctly
> [PR117790]
> 
> ping
> 
> > -----Original Message-----
> > From: Tamar Christina
> > Sent: Wednesday, January 15, 2025 2:07 PM
> > To: Alex Coplan <Alex.Coplan@arm.com>; gcc-patches@gcc.gnu.org
> > Cc: Richard Biener <rguenther@suse.de>; Jan Hubicka <hubicka@ucw.cz>
> > Subject: RE: [PATCH 1/4] vect: Set counts of early break exit blocks correctly
> > [PR117790]
> >
> > Ping
> >
> > > -----Original Message-----
> > > From: Alex Coplan <Alex.Coplan@arm.com>
> > > Sent: Monday, January 6, 2025 11:34 AM
> > > To: gcc-patches@gcc.gnu.org
> > > Cc: Richard Biener <rguenther@suse.de>; Jan Hubicka <hubicka@ucw.cz>;
> Tamar
> > > Christina <Tamar.Christina@arm.com>
> > > Subject: [PATCH 1/4] vect: Set counts of early break exit blocks correctly
> > > [PR117790]
> > >
> > > This adds missing code to correctly set the counts of the exit blocks we
> > > create when building the CFG for a vectorized early break loop.
> > >
> > > Tested as a series on aarch64-linux-gnu, arm-linux-gnueabihf, and
> > > x86_64-linux-gnu.  OK for trunk?
> > >
> > > Thanks,
> > > Alex
> > >
> > > gcc/ChangeLog:
> > >
> > > 	PR tree-optimization/117790
> > > 	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
> > > 	Set profile counts for {main,alt}_loop_exit_block.
> > > ---
> > >  gcc/tree-vect-loop-manip.cc | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
  
Jan Hubicka Feb. 4, 2025, 4:24 p.m. UTC | #4
> This adds missing code to correctly set the counts of the exit blocks we
> create when building the CFG for a vectorized early break loop.
> 
> Tested as a series on aarch64-linux-gnu, arm-linux-gnueabihf, and
> x86_64-linux-gnu.  OK for trunk?
> 
> Thanks,
> Alex
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/117790
> 	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
> 	Set profile counts for {main,alt}_loop_exit_block.
> ---
>  gcc/tree-vect-loop-manip.cc | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> index 5d1b70aea43..53d36eaa25f 100644
> --- a/gcc/tree-vect-loop-manip.cc
> +++ b/gcc/tree-vect-loop-manip.cc
> @@ -1686,6 +1686,16 @@ slpeel_tree_duplicate_loop_to_edge_cfg (class loop *loop, edge loop_exit,
>  
>  	  set_immediate_dominator (CDI_DOMINATORS, new_preheader,
>  				   loop->header);
> +
> +	  /* Fix up the profile counts of the new exit blocks.
> +	     main_loop_exit_block was created by duplicating the
> +	     preheader, so needs its count scaling according to the main
> +	     exit edge's probability.  The remaining count from the
> +	     preheader goes to the alt_loop_exit_block, since all
> +	     alternative exits have been redirected there.  */
> +	  main_loop_exit_block->count = loop_exit->count ();
> +	  alt_loop_exit_block->count
> +	    = preheader->count - main_loop_exit_block->count;

Reading the code, we orignaly have new_preheader that is split into
several pieces and loop exits are redirected to them and exit edges of
pieces are redirected to the last part?

In that case patch is OK.
Honza
  
Tamar Christina Feb. 5, 2025, 8:05 a.m. UTC | #5
> -----Original Message-----
> From: Jan Hubicka <hubicka@ucw.cz>
> Sent: Tuesday, February 4, 2025 4:25 PM
> To: Alex Coplan <Alex.Coplan@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Richard Biener <rguenther@suse.de>; Tamar
> Christina <Tamar.Christina@arm.com>
> Subject: Re: [PATCH 1/4] vect: Set counts of early break exit blocks correctly
> [PR117790]
> 
> > This adds missing code to correctly set the counts of the exit blocks we
> > create when building the CFG for a vectorized early break loop.
> >
> > Tested as a series on aarch64-linux-gnu, arm-linux-gnueabihf, and
> > x86_64-linux-gnu.  OK for trunk?
> >
> > Thanks,
> > Alex
> >
> > gcc/ChangeLog:
> >
> > 	PR tree-optimization/117790
> > 	* tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg):
> > 	Set profile counts for {main,alt}_loop_exit_block.
> > ---
> >  gcc/tree-vect-loop-manip.cc | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> 
> > diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> > index 5d1b70aea43..53d36eaa25f 100644
> > --- a/gcc/tree-vect-loop-manip.cc
> > +++ b/gcc/tree-vect-loop-manip.cc
> > @@ -1686,6 +1686,16 @@ slpeel_tree_duplicate_loop_to_edge_cfg (class loop
> *loop, edge loop_exit,
> >
> >  	  set_immediate_dominator (CDI_DOMINATORS, new_preheader,
> >  				   loop->header);
> > +
> > +	  /* Fix up the profile counts of the new exit blocks.
> > +	     main_loop_exit_block was created by duplicating the
> > +	     preheader, so needs its count scaling according to the main
> > +	     exit edge's probability.  The remaining count from the
> > +	     preheader goes to the alt_loop_exit_block, since all
> > +	     alternative exits have been redirected there.  */
> > +	  main_loop_exit_block->count = loop_exit->count ();
> > +	  alt_loop_exit_block->count
> > +	    = preheader->count - main_loop_exit_block->count;
> 
> Reading the code, we orignaly have new_preheader that is split into
> several pieces and loop exits are redirected to them and exit edges of
> pieces are redirected to the last part?
> 

Indeed, All alternate exits are redirected to the same block, as they share
the same induction values, that block and the main exit are then directed to
a join block which is created just above the guard block for the epilogue.


> In that case patch is OK.

Thanks!
Tamar

> Honza
  

Patch

diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index 5d1b70aea43..53d36eaa25f 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -1686,6 +1686,16 @@  slpeel_tree_duplicate_loop_to_edge_cfg (class loop *loop, edge loop_exit,
 
 	  set_immediate_dominator (CDI_DOMINATORS, new_preheader,
 				   loop->header);
+
+	  /* Fix up the profile counts of the new exit blocks.
+	     main_loop_exit_block was created by duplicating the
+	     preheader, so needs its count scaling according to the main
+	     exit edge's probability.  The remaining count from the
+	     preheader goes to the alt_loop_exit_block, since all
+	     alternative exits have been redirected there.  */
+	  main_loop_exit_block->count = loop_exit->count ();
+	  alt_loop_exit_block->count
+	    = preheader->count - main_loop_exit_block->count;
 	}
 
       /* Adjust the epilog loop PHI entry values to continue iteration.