[PATCH] gimple-iterator: Some gsi_safe_insert_*before fixes

[PATCH] gimple-iterator: Some gsi_safe_insert_*before fixes

[Jakub Jelinek]

Hi!

When trying to use the gsi_safe_insert*before APIs in bitint lowering,
I've discovered 3 issues and the following patch addresses those:

1) both split_block and split_edge update CDI_DOMINATORS if they are
   available, but because edge_before_returns_twice_call first splits
   and then adds an extra EDGE_ABNORMAL edge and then removes another
   one, the immediate dominators of both the new bb and the bb with
   returns_twice call need to change
2) the new EDGE_ABNORMAL edge had uninitialized probability; this patch
   copies the probability from the edge that is going to be removed
   and similarly copies other flags (EDGE_EXECUTABLE, EDGE_DFS_BACK,
   EDGE_IRREDUCIBLE_LOOP etc.)
3) if edge_before_returns_twice_call splits a block, then the bb with
   returns_twice call changes, so the gimple_stmt_iterator for it is
   no longer accurate, it points to the right statement, but gsi_bb
   and gsi_seq are no longer correct; the patch updates it

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-03-14  Jakub Jelinek  <jakub@redhat.com>

	* gimple-iterator.cc (edge_before_returns_twice_call): Copy all
	flags and probability from ad_edge to e edge.  If CDI_DOMINATORS
	are computed, recompute immediate dominator of other_edge->src
	and other_edge->dest.
	(gsi_safe_insert_before, gsi_safe_insert_seq_before): Update *iter
	for the returns_twice call case to the gsi_for_stmt (stmt) to deal
	with update it for bb splitting.

--- gcc/gimple-iterator.cc.jj	2024-03-13 13:03:08.073117732 +0100
+++ gcc/gimple-iterator.cc	2024-03-13 15:16:45.294317700 +0100
@@ -997,7 +997,18 @@ edge_before_returns_twice_call (basic_bl
 	  add_phi_arg (new_phi, gimple_phi_arg_def_from_edge (phi, ad_edge),
 		       e, gimple_phi_arg_location_from_edge (phi, ad_edge));
 	}
+      e->flags = ad_edge->flags;
+      e->probability = ad_edge->probability;
       remove_edge (ad_edge);
+      if (dom_info_available_p (CDI_DOMINATORS))
+	{
+	  set_immediate_dominator (CDI_DOMINATORS, other_edge->src,
+				   recompute_dominator (CDI_DOMINATORS,
+							other_edge->src));
+	  set_immediate_dominator (CDI_DOMINATORS, other_edge->dest,
+				   recompute_dominator (CDI_DOMINATORS,
+							other_edge->dest));
+	}
     }
   return other_edge;
 }
@@ -1045,6 +1056,7 @@ gsi_safe_insert_before (gimple_stmt_iter
       if (new_bb)
 	e = single_succ_edge (new_bb);
       adjust_before_returns_twice_call (e, g);
+      *iter = gsi_for_stmt (stmt);
     }
   else
     gsi_insert_before (iter, g, GSI_SAME_STMT);
@@ -1075,6 +1087,7 @@ gsi_safe_insert_seq_before (gimple_stmt_
 	  if (g == l)
 	    break;
 	}
+      *iter = gsi_for_stmt (stmt);
     }
   else
     gsi_insert_seq_before (iter, seq, GSI_SAME_STMT);

	Jakub

Re: [PATCH] gimple-iterator: Some gsi_safe_insert_*before fixes

[Richard Biener]

On Thu, 14 Mar 2024, Jakub Jelinek wrote:

> Hi!
> 
> When trying to use the gsi_safe_insert*before APIs in bitint lowering,
> I've discovered 3 issues and the following patch addresses those:
> 
> 1) both split_block and split_edge update CDI_DOMINATORS if they are
>    available, but because edge_before_returns_twice_call first splits
>    and then adds an extra EDGE_ABNORMAL edge and then removes another
>    one, the immediate dominators of both the new bb and the bb with
>    returns_twice call need to change
> 2) the new EDGE_ABNORMAL edge had uninitialized probability; this patch
>    copies the probability from the edge that is going to be removed
>    and similarly copies other flags (EDGE_EXECUTABLE, EDGE_DFS_BACK,
>    EDGE_IRREDUCIBLE_LOOP etc.)
> 3) if edge_before_returns_twice_call splits a block, then the bb with
>    returns_twice call changes, so the gimple_stmt_iterator for it is
>    no longer accurate, it points to the right statement, but gsi_bb
>    and gsi_seq are no longer correct; the patch updates it
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK

> 2024-03-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* gimple-iterator.cc (edge_before_returns_twice_call): Copy all
> 	flags and probability from ad_edge to e edge.  If CDI_DOMINATORS
> 	are computed, recompute immediate dominator of other_edge->src
> 	and other_edge->dest.
> 	(gsi_safe_insert_before, gsi_safe_insert_seq_before): Update *iter
> 	for the returns_twice call case to the gsi_for_stmt (stmt) to deal
> 	with update it for bb splitting.
> 
> --- gcc/gimple-iterator.cc.jj	2024-03-13 13:03:08.073117732 +0100
> +++ gcc/gimple-iterator.cc	2024-03-13 15:16:45.294317700 +0100
> @@ -997,7 +997,18 @@ edge_before_returns_twice_call (basic_bl
>  	  add_phi_arg (new_phi, gimple_phi_arg_def_from_edge (phi, ad_edge),
>  		       e, gimple_phi_arg_location_from_edge (phi, ad_edge));
>  	}
> +      e->flags = ad_edge->flags;
> +      e->probability = ad_edge->probability;
>        remove_edge (ad_edge);
> +      if (dom_info_available_p (CDI_DOMINATORS))
> +	{
> +	  set_immediate_dominator (CDI_DOMINATORS, other_edge->src,
> +				   recompute_dominator (CDI_DOMINATORS,
> +							other_edge->src));
> +	  set_immediate_dominator (CDI_DOMINATORS, other_edge->dest,
> +				   recompute_dominator (CDI_DOMINATORS,
> +							other_edge->dest));
> +	}
>      }
>    return other_edge;
>  }
> @@ -1045,6 +1056,7 @@ gsi_safe_insert_before (gimple_stmt_iter
>        if (new_bb)
>  	e = single_succ_edge (new_bb);
>        adjust_before_returns_twice_call (e, g);
> +      *iter = gsi_for_stmt (stmt);
>      }
>    else
>      gsi_insert_before (iter, g, GSI_SAME_STMT);
> @@ -1075,6 +1087,7 @@ gsi_safe_insert_seq_before (gimple_stmt_
>  	  if (g == l)
>  	    break;
>  	}
> +      *iter = gsi_for_stmt (stmt);
>      }
>    else
>      gsi_insert_seq_before (iter, seq, GSI_SAME_STMT);
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)