diff mbox series

IPA: Optionally allow double costing, restoring GCC 10 behavior

Message ID patch-14145-tamar@arm.com
State New
Headers show
Series IPA: Optionally allow double costing, restoring GCC 10 behavior | expand

Commit Message

Tamar Christina Feb. 10, 2021, 2:32 p.m. UTC 
Hi Honza and Martin,

As explained in PR98782 the problem is that the behavior of IRA is quite tightly
bound to the frequencies that IPA puts out.  With the change introduced in
g:1118a3ff9d3ad6a64bba25dc01e7703325e23d92 some, but not all edge predictions
changed.  This introduces a local problem which is impossible to fix using
global flags or using any hooks to IRA.

I propose this new parameter ipa-cp-allow-multi-edge-costing as a temporary stop
gap as something that is safe for stage4.

This allows us to opt in to allow the double edge costing, and buys us some time
to properly look at RA in GCC 12 to see if there's a proper solution for this
problem.

Using --param ipa-cp-allow-multi-edge-costing=1 allows us to regain all the loses
compared to GCC 10, but also thanks to Martin's patches we get a tiny extra gain
as well.

Would this be an acceptable stop-gap measure for the both of you?

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	PR rtl-optimization/98782
	* params.opt (ipa-cp-allow-multi-edge-costing): New.
	* predict.c (maybe_predict_edge): Use it.

--- inline copy of patch -- 
diff --git a/gcc/params.opt b/gcc/params.opt
index dff8a331f82e44c1b90e5b9f88ffc61e84f03d2d..f54eba2836f5d3a9b66489e3766f3d45eeaf5952 100644


--

Comments

Richard Biener Feb. 10, 2021, 3:52 p.m. UTC | #1 
On Wed, Feb 10, 2021 at 4:16 PM Tamar Christina via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi Honza and Martin,
>
> As explained in PR98782 the problem is that the behavior of IRA is quite tightly
> bound to the frequencies that IPA puts out.  With the change introduced in
> g:1118a3ff9d3ad6a64bba25dc01e7703325e23d92 some, but not all edge predictions
> changed.  This introduces a local problem which is impossible to fix using
> global flags or using any hooks to IRA.
>
> I propose this new parameter ipa-cp-allow-multi-edge-costing as a temporary stop
> gap as something that is safe for stage4.
>
> This allows us to opt in to allow the double edge costing, and buys us some time
> to properly look at RA in GCC 12 to see if there's a proper solution for this
> problem.
>
> Using --param ipa-cp-allow-multi-edge-costing=1 allows us to regain all the loses
> compared to GCC 10, but also thanks to Martin's patches we get a tiny extra gain
> as well.
>
> Would this be an acceptable stop-gap measure for the both of you?

With the --param defaulted to off this is solely a knob to get better benchmark
numbers?  Sorry, but GCC isn't a benchmark compiler.

Btw, even such kind of --params deserve documentation.  I guess if it
then states "Use this to speedup SPEC CPU 123.xyz" then the switch becomes
disallowed by SPEC rules even for PEAK, no?

Richard.

> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
>         PR rtl-optimization/98782
>         * params.opt (ipa-cp-allow-multi-edge-costing): New.
>         * predict.c (maybe_predict_edge): Use it.
>
> --- inline copy of patch --
> diff --git a/gcc/params.opt b/gcc/params.opt
> index dff8a331f82e44c1b90e5b9f88ffc61e84f03d2d..f54eba2836f5d3a9b66489e3766f3d45eeaf5952 100644
> --- a/gcc/params.opt
> +++ b/gcc/params.opt
> @@ -277,6 +277,10 @@ The size of translation unit that IPA-CP pass considers large.
>  Common Joined UInteger Var(param_ipa_cp_value_list_size) Init(8) Param Optimization
>  Maximum size of a list of values associated with each parameter for interprocedural constant propagation.
>
> +-param=ipa-cp-allow-multi-edge-costing=
> +Common Joined UInteger Var(param_ipa_cp_allow_multi_edge_costing) Init(0) IntegerRange(0, 1) Param Optimization
> +Allow double prediction to happen during IPA edge analysis.
> +
>  -param=ipa-jump-function-lookups=
>  Common Joined UInteger Var(param_ipa_jump_function_lookups) Init(8) Param Optimization
>  Maximum number of statements visited during jump function offset discovery.
> diff --git a/gcc/predict.c b/gcc/predict.c
> index d0a8e5f8e04fc3d1ec770602589299b0a30b3b59..c15dd92310dfe77bedc50c027d000833c0a92315 100644
> --- a/gcc/predict.c
> +++ b/gcc/predict.c
> @@ -3171,16 +3171,22 @@ maybe_predict_edge (edge e, enum br_predictor pred, enum prediction taken)
>  {
>    if (edge_predicted_by_p (e, pred, taken))
>      return;
> -  if (pred == PRED_LOOP_GUARD
> -      && edge_predicted_by_p (e, PRED_LOOP_GUARD_WITH_RECURSION, taken))
> -    return;
> -  /* Consider PRED_LOOP_GUARD_WITH_RECURSION superrior to LOOP_GUARD.  */
> -  if (pred == PRED_LOOP_GUARD_WITH_RECURSION)
> +
> +  /* Allow double prediction in certain circumstances. See PR98782.  */
> +  if (!param_ipa_cp_allow_multi_edge_costing)
>      {
> -      edge_prediction **preds = bb_predictions->get (e->src);
> -      if (preds)
> -       filter_predictions (preds, not_loop_guard_equal_edge_p, e);
> +      if (pred == PRED_LOOP_GUARD
> +         && edge_predicted_by_p (e, PRED_LOOP_GUARD_WITH_RECURSION, taken))
> +       return;
> +      /* Consider PRED_LOOP_GUARD_WITH_RECURSION superrior to LOOP_GUARD.  */
> +      if (pred == PRED_LOOP_GUARD_WITH_RECURSION)
> +       {
> +         edge_prediction **preds = bb_predictions->get (e->src);
> +         if (preds)
> +           filter_predictions (preds, not_loop_guard_equal_edge_p, e);
> +       }
>      }
> +
>    predict_edge_def (e, pred, taken);
>  }
>  /* Predict edges to successors of CUR whose sources are not postdominated by
>
>
> --
diff mbox series

Patch

diff --git a/gcc/params.opt b/gcc/params.opt
index dff8a331f82e44c1b90e5b9f88ffc61e84f03d2d..f54eba2836f5d3a9b66489e3766f3d45eeaf5952 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -277,6 +277,10 @@  The size of translation unit that IPA-CP pass considers large.
 Common Joined UInteger Var(param_ipa_cp_value_list_size) Init(8) Param Optimization
 Maximum size of a list of values associated with each parameter for interprocedural constant propagation.
 
+-param=ipa-cp-allow-multi-edge-costing=
+Common Joined UInteger Var(param_ipa_cp_allow_multi_edge_costing) Init(0) IntegerRange(0, 1) Param Optimization
+Allow double prediction to happen during IPA edge analysis.
+
 -param=ipa-jump-function-lookups=
 Common Joined UInteger Var(param_ipa_jump_function_lookups) Init(8) Param Optimization
 Maximum number of statements visited during jump function offset discovery.
diff --git a/gcc/predict.c b/gcc/predict.c
index d0a8e5f8e04fc3d1ec770602589299b0a30b3b59..c15dd92310dfe77bedc50c027d000833c0a92315 100644
--- a/gcc/predict.c
+++ b/gcc/predict.c
@@ -3171,16 +3171,22 @@  maybe_predict_edge (edge e, enum br_predictor pred, enum prediction taken)
 {
   if (edge_predicted_by_p (e, pred, taken))
     return;
-  if (pred == PRED_LOOP_GUARD
-      && edge_predicted_by_p (e, PRED_LOOP_GUARD_WITH_RECURSION, taken))
-    return;
-  /* Consider PRED_LOOP_GUARD_WITH_RECURSION superrior to LOOP_GUARD.  */
-  if (pred == PRED_LOOP_GUARD_WITH_RECURSION)
+
+  /* Allow double prediction in certain circumstances. See PR98782.  */
+  if (!param_ipa_cp_allow_multi_edge_costing)
     {
-      edge_prediction **preds = bb_predictions->get (e->src);
-      if (preds)
-	filter_predictions (preds, not_loop_guard_equal_edge_p, e);
+      if (pred == PRED_LOOP_GUARD
+	  && edge_predicted_by_p (e, PRED_LOOP_GUARD_WITH_RECURSION, taken))
+	return;
+      /* Consider PRED_LOOP_GUARD_WITH_RECURSION superrior to LOOP_GUARD.  */
+      if (pred == PRED_LOOP_GUARD_WITH_RECURSION)
+	{
+	  edge_prediction **preds = bb_predictions->get (e->src);
+	  if (preds)
+	    filter_predictions (preds, not_loop_guard_equal_edge_p, e);
+	}
     }
+
   predict_edge_def (e, pred, taken);
 }
 /* Predict edges to successors of CUR whose sources are not postdominated by