On Sat, Jan 11, 2025 at 2:10 AM Richard Biener <richard.guent...@gmail.com> wrote: > > > > > Am 11.01.2025 um 09:49 schrieb Andrew Pinski <quic_apin...@quicinc.com>: > > > > In this case, early phiopt would get rid of the user provided predicator > > for hot/cold as it would remove the basic blocks. The easiest and best > > option is > > for early phi-opt don't do phi-opt if the middle basic-block(s) have either > > a hot or cold predict statement. Then after inlining, jump threading will > > most likely > > happen and that will keep around the predictor. > > > > Note this only needs to be done for match_simplify_replacement and not the > > other > > phi-opt functions because currently only match_simplify_replacement is able > > to skip > > middle bb with predicator statements in it. > > > > OK? Bootstrapped and tested on x86_64-linux-gnu. > > Hmm, I think we still want to allow abs/min/max replacements early I think. > > What kind of replacement is performed in the problematic case?
Replace `a ? true : false` with simple `a`. So should we just reject that if early and the middle contains a hot/cold predictor? That is the only case where jump threading will most likely happen after inlining too. I will work on that tomorrow. > > > > > PR tree-optimization/117935 > > > > gcc/ChangeLog: > > > > * tree-ssa-phiopt.cc (contains_hot_cold_predict): New function. > > (match_simplify_replacement): Return early if early_p and one of > > the middle bb(s) have a hot/cold predict statement. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/predict-24.c: New test. > > > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com> > > --- > > gcc/testsuite/gcc.dg/predict-24.c | 24 +++++++++++++++++++++ > > gcc/tree-ssa-phiopt.cc | 35 +++++++++++++++++++++++++++++++ > > 2 files changed, 59 insertions(+) > > create mode 100644 gcc/testsuite/gcc.dg/predict-24.c > > > > diff --git a/gcc/testsuite/gcc.dg/predict-24.c > > b/gcc/testsuite/gcc.dg/predict-24.c > > new file mode 100644 > > index 00000000000..b2c8a77c323 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/predict-24.c > > @@ -0,0 +1,24 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-profile_estimate" } */ > > +/* PR tree-optimization/117935 */ > > + > > +static inline bool has_value(bool b) > > +{ > > + if (b) > > + { > > + [[gnu::hot, gnu::unused]] label1: > > + return true; > > + } > > + else > > + return false; > > +} > > +/* The hot label should last until it gets inlined into value_or and > > jump_threaded. */ > > +int value_or(bool b, int def0, int def1) > > +{ > > + if (has_value(b)) > > + return def0; > > + else > > + return def1; > > +} > > +/* { dg-final { scan-tree-dump-times "first match heuristics: 90.00%" 2 > > "profile_estimate"} } */ > > +/* { dg-final { scan-tree-dump-times "hot label heuristics of edge > > \[0-9\]+->\[0-9]+: 90.00%" 2 "profile_estimate"} } */ > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > > index 64d3ba9e160..8fed3eadbb0 100644 > > --- a/gcc/tree-ssa-phiopt.cc > > +++ b/gcc/tree-ssa-phiopt.cc > > @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3. If not see > > #include "tree-ssa-dce.h" > > #include "calls.h" > > #include "tree-ssa-loop-niter.h" > > +#include "gimple-predict.h" > > > > /* Return the singleton PHI in the SEQ of PHIs for edges E0 and E1. */ > > > > @@ -916,6 +917,27 @@ auto_flow_sensitive::~auto_flow_sensitive () > > p.second.restore (p.first); > > } > > > > +/* Returns true if BB contains an user provided predictor > > + (PRED_HOT_LABEL/PRED_COLD_LABEL). */ > > + > > +static bool > > +contains_hot_cold_predict (basic_block bb) > > +{ > > + gimple_stmt_iterator gsi; > > + gsi = gsi_start_nondebug_after_labels_bb (bb); > > + for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi)) > > + { > > + gimple *s = gsi_stmt (gsi); > > + if (gimple_code (s) != GIMPLE_PREDICT) > > + continue; > > + auto predict = gimple_predict_predictor (s); > > + if (predict == PRED_HOT_LABEL > > + || predict == PRED_COLD_LABEL) > > + return true; > > + } > > + return false; > > +} > > + > > /* The function match_simplify_replacement does the main work of doing the > > replacement using match and simplify. Return true if the replacement > > is done. > > Otherwise return false. > > @@ -953,6 +975,19 @@ match_simplify_replacement (basic_block cond_bb, > > basic_block middle_bb, > > stmt_to_move_alt)) > > return false; > > > > + /* For early phiopt, we don't want to lose user generated predictors, > > + that is if either middle bb contains either a hot or cold label > > predict, > > + the phiopt should be skipped. */ > > + if (early_p) > > + { > > + if (contains_hot_cold_predict (middle_bb)) > > + return false; > > + if (threeway_p > > + && middle_bb != middle_bb_alt > > + && contains_hot_cold_predict (middle_bb_alt)) > > + return false; > > + } > > + > > /* Do not make conditional undefs unconditional. */ > > if ((TREE_CODE (arg0) == SSA_NAME > > && ssa_name_maybe_undef_p (arg0)) > > -- > > 2.43.0 > >