On Mon, Jul 13, 2026 at 5:41 AM <[email protected]> wrote: > > From: Kyrylo Tkachov <[email protected]> > > A comparison mask such as > > m = cmp ? -1 : 0 > > can remain a constant-pair PHI. Later equality tests of m then remain > separate diamonds, allowing backward jump threading to duplicate the > chain instead of exposing a shared comparison for if-conversion. > > Teach phiopt_early_allow to accept the exact comparison-mask forms CMP, > (convert)CMP, -CMP, -(convert)CMP, (convert)-CMP and > (convert)-(convert)CMP. > > Validate each form by walking the use-def chain from the final result. > Definitions belonging to the not-yet-inserted match sequence have no > basic block. GIMPLE_NOP default definitions are treated as > pre-existing values. Any other generated definition on the used > chain, including a call, remains visible and causes a nonmatching shape > to be rejected. Generated statements no longer used by the result are > not visited. > > Classify the result-defining final statement when preserving user hot > and cold predictors. This covers multi-statement conversion and > negation masks without scanning the sequence. > > Do not materialize a mask for a diamond whose join or arm is entered by > another path when either the generated mask has multiple statements or > the condition block contains a PHI or a preparation statement. Such > cases leave a residual multi-statement middle that late phiopt cannot > move and can prevent tree ifcombine from forming a conditional-compare > chain. A bare comparison from a condition-only block remains allowed > because its residual middle is movable. > > Keep tests intended to exercise later prediction, threading, VRP or > diagnostic behavior on their original shapes by disabling phiopt1. > Add direct coverage for call-containing goto prediction, hot and cold > predictors on multi-statement masks, comparison-mask materialization, > and the bare-comparison embedded-diamond case. Retain the existing > conditional-compare chain when its condition block contains a > preparation statement. > > Together with the following merge-diamonds patch, this makes > 731.astcenc_r about 20% faster on AArch64 at -Ofast -flto=auto. > > Bootstrapped and tested on aarch64-none-linux-gnu. > > gcc/ChangeLog: > > PR tree-optimization/125672 > PR tree-optimization/126043 > * tree-ssa-phiopt.cc (phiopt_seq_def_stmt): New function. > (phiopt_skip_mask_conversion): New function. > (phiopt_self_contained_cmp_p): New function. > (phiopt_early_allow): Accept the supported comparison-mask forms, > validating them along the result's use-def chain. > (match_simplify_replacement): Classify the result-defining > statement when preserving user predictors. Reject materializing a > mask in an embedded diamond when its generated sequence has multiple > statements or its condition block contains additional content. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/125672 > PR tree-optimization/126043 > * g++.dg/predict-loop-exit-1.C: Disable phiopt1. > * g++.dg/predict-loop-exit-2.C: Likewise. > * g++.dg/predict-loop-exit-3.C: Likewise. > * gcc.dg/auto-init-uninit-15.c: Expect the warning at the use in > bar. > * gcc.dg/pr109071_6.c: Disable phiopt1. > * gcc.dg/predict-15.c: Likewise. > * gcc.dg/predict-15a.c: New test. > * gcc.dg/predict-26.c: New test. > * gcc.dg/tree-ssa/pr125672-1.c: New test. > * gcc.dg/tree-ssa/pr125672-4.c: New test. > * gcc.dg/tree-ssa/pr21090-2.c: New test. > * gcc.dg/tree-ssa/pr21090.c: Disable phiopt1. > * gcc.dg/tree-ssa/pr68198.c: Likewise. > > Signed-off-by: Kyrylo Tkachov <[email protected]> > --- > gcc/testsuite/g++.dg/predict-loop-exit-1.C | 4 +- > gcc/testsuite/g++.dg/predict-loop-exit-2.C | 4 +- > gcc/testsuite/g++.dg/predict-loop-exit-3.C | 4 +- > gcc/testsuite/gcc.dg/auto-init-uninit-15.c | 11 +- > gcc/testsuite/gcc.dg/pr109071_6.c | 5 +- > gcc/testsuite/gcc.dg/predict-15.c | 4 +- > gcc/testsuite/gcc.dg/predict-15a.c | 24 ++++ > gcc/testsuite/gcc.dg/predict-26.c | 50 +++++++ > gcc/testsuite/gcc.dg/tree-ssa/pr125672-1.c | 52 +++++++ > gcc/testsuite/gcc.dg/tree-ssa/pr125672-4.c | 21 +++ > gcc/testsuite/gcc.dg/tree-ssa/pr21090-2.c | 27 ++++ > gcc/testsuite/gcc.dg/tree-ssa/pr21090.c | 5 +- > gcc/testsuite/gcc.dg/tree-ssa/pr68198.c | 4 +- > gcc/tree-ssa-phiopt.cc | 150 ++++++++++++++++++--- > 14 files changed, 335 insertions(+), 30 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/predict-15a.c > create mode 100644 gcc/testsuite/gcc.dg/predict-26.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr125672-1.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr125672-4.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr21090-2.c > > diff --git a/gcc/testsuite/g++.dg/predict-loop-exit-1.C > b/gcc/testsuite/g++.dg/predict-loop-exit-1.C > index 47bdf5182c1..a0081411c88 100644 > --- a/gcc/testsuite/g++.dg/predict-loop-exit-1.C > +++ b/gcc/testsuite/g++.dg/predict-loop-exit-1.C > @@ -1,5 +1,7 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-profile_estimate -fdisable-tree-ethread" } > */ > +/* { dg-options "-O2 -fdump-tree-profile_estimate -fdisable-tree-ethread > -fdisable-tree-phiopt1" } */ > +/* Disable phiopt1 so the short-circuit 0/1 PHI reaches profile estimation > + and exercises the extra loop exit predictor. */ > > int g; > int foo(); > diff --git a/gcc/testsuite/g++.dg/predict-loop-exit-2.C > b/gcc/testsuite/g++.dg/predict-loop-exit-2.C > index 15b63eeb0fa..8e24e7f317d 100644 > --- a/gcc/testsuite/g++.dg/predict-loop-exit-2.C > +++ b/gcc/testsuite/g++.dg/predict-loop-exit-2.C > @@ -1,5 +1,7 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-profile_estimate -fdisable-tree-ethread" } > */ > +/* { dg-options "-O2 -fdump-tree-profile_estimate -fdisable-tree-ethread > -fdisable-tree-phiopt1" } */ > +/* Disable phiopt1 so the short-circuit 0/1 PHI reaches profile estimation > + and exercises the extra loop exit predictor. */ > > int g; > int foo(); > diff --git a/gcc/testsuite/g++.dg/predict-loop-exit-3.C > b/gcc/testsuite/g++.dg/predict-loop-exit-3.C > index 8d417422b60..561ed786fb6 100644 > --- a/gcc/testsuite/g++.dg/predict-loop-exit-3.C > +++ b/gcc/testsuite/g++.dg/predict-loop-exit-3.C > @@ -1,5 +1,7 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-profile_estimate -fdisable-tree-ethread" } > */ > +/* { dg-options "-O2 -fdump-tree-profile_estimate -fdisable-tree-ethread > -fdisable-tree-phiopt1" } */ > +/* Disable phiopt1 so the short-circuit 0/1 PHI reaches profile estimation > + and exercises the extra loop exit predictor. */ > > int g; > int foo(); > diff --git a/gcc/testsuite/gcc.dg/auto-init-uninit-15.c > b/gcc/testsuite/gcc.dg/auto-init-uninit-15.c > index 121f0cff274..4c0c43c94c7 100644 > --- a/gcc/testsuite/gcc.dg/auto-init-uninit-15.c > +++ b/gcc/testsuite/gcc.dg/auto-init-uninit-15.c > @@ -1,16 +1,13 @@ > /* PR tree-optimization/17506 > - We issue an uninitialized variable warning at a wrong location at > - line 11, which is very confusing. Make sure we print out a note to > - make it less confusing. (not xfailed alternative) > - But it is of course ok if we warn in bar about uninitialized use > - of j. (not xfailed alternative) */ > + The warning is issued at the use of j in bar, with a note pointing to > + its declaration. */ > /* { dg-do compile } */ > /* { dg-options "-O1 -Wuninitialized -ftrivial-auto-var-init=zero" } */ > > inline int > foo (int i) > { > - if (i) /* { dg-warning "used uninitialized" } */ > + if (i) > return 1; > return 0; > } > @@ -21,6 +18,6 @@ void > bar (void) > { > int j; /* { dg-message "note: 'j' was declared here" "" } */ > - for (; foo (j); ++j) /* { dg-warning "'j' is used uninitialized" "" { > xfail *-*-* } } */ > + for (; foo (j); ++j) /* { dg-warning "'j' is used uninitialized" } */ > baz (); > } > diff --git a/gcc/testsuite/gcc.dg/pr109071_6.c > b/gcc/testsuite/gcc.dg/pr109071_6.c > index eddf15b350c..6062882583f 100644 > --- a/gcc/testsuite/gcc.dg/pr109071_6.c > +++ b/gcc/testsuite/gcc.dg/pr109071_6.c > @@ -1,7 +1,10 @@ > /* PR tree-optimization/109071 need more context for -Warray-bounds warnings > due to code duplication from jump threading. > test case is from PR117179, which is a duplication of PR109071. */ > -/* { dg-options "-O2 -Warray-bounds -fdiagnostics-show-context=1" } */ > +/* { dg-options "-O2 -Warray-bounds -fdiagnostics-show-context=1 > -fdisable-tree-phiopt1" } */ > +/* phiopt1 is disabled: early phiopt now materializes setval_internal's > + comparison, so no threaded path with the out-of-bounds range reaches > + the array access and no warning is issued at all. */ > /* { dg-additional-options "-fdiagnostics-show-line-numbers > -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */ > /* { dg-enable-nn-line-numbers "" } */ > const char* commands[] = {"a", "b"}; > diff --git a/gcc/testsuite/gcc.dg/predict-15.c > b/gcc/testsuite/gcc.dg/predict-15.c > index 2a8c3ea8597..9a25537c9b6 100644 > --- a/gcc/testsuite/gcc.dg/predict-15.c > +++ b/gcc/testsuite/gcc.dg/predict-15.c > @@ -1,5 +1,7 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-profile_estimate" } */ > +/* { dg-options "-O2 -fdump-tree-profile_estimate -fdisable-tree-phiopt1" } > */ > +/* phiopt1 is disabled: early phiopt now materializes the comparison > + and no goto edge remains to predict. */ > > int main(int argc, char **argv) > { > diff --git a/gcc/testsuite/gcc.dg/predict-15a.c > b/gcc/testsuite/gcc.dg/predict-15a.c > new file mode 100644 > index 00000000000..bb6e899e98b > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/predict-15a.c > @@ -0,0 +1,24 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-profile_estimate" } */ > +/* Like predict-15.c, but with a call in both paths: early phiopt cannot > + materialize the comparison, so the user goto edge and its predictor > + remain (no -fdisable-tree-phiopt1 needed). */ > + > +void g (int); > + > +int main (int argc, char **argv) > +{ > + if (argc == 123) > + goto exit; > + else > + { > + g (0); > + return 0; > + } > + > +exit: > + g (1); > + return 1; > +} > + > +/* { dg-final { scan-tree-dump "goto heuristics of edge" "profile_estimate" > } } */ > diff --git a/gcc/testsuite/gcc.dg/predict-26.c > b/gcc/testsuite/gcc.dg/predict-26.c > new file mode 100644 > index 00000000000..8b0e18fc2e4 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/predict-26.c > @@ -0,0 +1,50 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-phiopt1-details > -fdump-tree-profile_estimate" } */ > +/* PR tree-optimization/126043 */ > + > +/* The unannotated diamond is materialized as -(int) CMP by phiopt1. > + Diamonds carrying user hot or cold predictors remain until profile > + estimation. */ > + > +int > +hot_mask (int a, int b) > +{ > + int mask; > + if (a > b) > + { > + hot: __attribute__ ((hot)) > + mask = -1; > + } > + else > + mask = 0; > + return mask; > +} > + > +int > +cold_mask (int a, int b) > +{ > + int mask; > + if (a > b) > + { > + cold: __attribute__ ((cold)) > + mask = -1; > + } > + else > + mask = 0; > + return mask; > +} > + > +int > +plain_mask (int a, int b) > +{ > + int mask; > + if (a > b) > + mask = -1; > + else > + mask = 0; > + return mask; > +} > + > +/* { dg-final { scan-tree-dump-times "hot label heuristics" 1 > "profile_estimate" } } */ > +/* { dg-final { scan-tree-dump-times "cold label heuristics" 1 > "profile_estimate" } } */ > +/* { dg-final { scan-tree-dump-times "converted to straightline code" 1 > "phiopt1" } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr125672-1.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr125672-1.c > new file mode 100644 > index 00000000000..9640d184336 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr125672-1.c > @@ -0,0 +1,52 @@ > +/* Early phiopt materializes a comparison mask "cmp ? CST1 : CST2" as > + -(int)cmp / (int)cmp, exposing its comparison. An equality test of the > + mask ("m == C") then folds to the comparison itself, so the mask > + disappears and diamonds controlled by the same mask share an identical > + condition (a prerequisite for pass_merge_diamonds). */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-phiopt1" } */ > + > +/* -(int)cmp all-ones mask tested against a constant: the test folds to the > + comparison. */ > +int > +neg_masktest (int a, int b, int x, int y) > +{ > + int m; > + if (a > b) > + m = -1; > + else > + m = 0; > + return (m == -1) ? x : y; > +} > + > +/* (int)cmp boolean mask tested against a constant (the convert case). */ > +int > +cvt_masktest (int a, int b, int x, int y) > +{ > + int m; > + if (a > b) > + m = 1; > + else > + m = 0; > + return (m == 1) ? x : y; > +} > + > +/* A mask consumed by other operators is materialized too. */ > +int > +mask_algebra (int a, int b) > +{ > + int m; > + if (a > b) > + m = -1; > + else > + m = 0; > + return m & 7; > +} > + > +/* No constant-pair mask PHI survives and the equality tests fold away. > + Check both argument orders of the arithmetic mask PHI. */ > +/* { dg-final { scan-tree-dump-not "= PHI <-1\\(" "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-not "= PHI <0\\(" "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-not "== -1" "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-not "== 1" "phiopt1" } } */ > +/* { dg-final { scan-tree-dump "& 7" "phiopt1" } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr125672-4.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr125672-4.c > new file mode 100644 > index 00000000000..7c9266ca7b2 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr125672-4.c > @@ -0,0 +1,21 @@ > +/* A bare-comparison mask from a condition-only block may be materialized > + even for a diamond whose join has other predecessors. The residual middle > + stays single-statement, which late phiopt can still rewire. */ > +/* { dg-do compile } */ > +/* { dg-options "-O1 -fno-thread-jumps -fdisable-tree-ifcombine > -fdump-tree-phiopt1 -fdump-tree-optimized" } */ > + > +_Bool f (int a) > +{ > + if (a < 10) > + return 1; > + if (a < 3) > + return 1; > + return 0; > +} > + > +/* The second condition is materialized as a bare comparison mask by early > + phiopt. Only the first condition remains a branch. */ > +/* { dg-final { scan-tree-dump-times "if \\(" 1 "phiopt1" } } */ > + > +/* Late phiopt removes the remaining branch. */ > +/* { dg-final { scan-tree-dump-not "if \\(" "optimized" } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr21090-2.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr21090-2.c > new file mode 100644 > index 00000000000..d3792d4314f > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr21090-2.c > @@ -0,0 +1,27 @@ > +/* With early phiopt lowering "p != 0 ? 1 : 0" to > + a boolean mask, the non-null test of a PHI of two object addresses folds > + away entirely, without needing VRP's predicate folding (compare > + pr21090.c). */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-thread-jumps -fdelete-null-pointer-checks > -fdump-tree-optimized" } */ > + > +int g, h; > + > +int > +foo (int a) > +{ > + int *p; > + > + if (a) > + p = &g; > + else > + p = &h; > + > + if (p != 0) > + return 1; > + else > + return 0; > +} > + > +/* { dg-final { scan-tree-dump "return 1;" "optimized" { target { ! > keeps_null_pointer_checks } } } } */ > +/* { dg-final { scan-tree-dump-not "PHI" "optimized" { target { ! > keeps_null_pointer_checks } } } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr21090.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr21090.c > index 92a87688601..7761b39a459 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/pr21090.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr21090.c > @@ -1,5 +1,8 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fno-thread-jumps -fdisable-tree-evrp -fdump-tree-vrp1 > -fdelete-null-pointer-checks" } */ > +/* { dg-options "-O2 -fno-thread-jumps -fdisable-tree-evrp -fdump-tree-vrp1 > -fdelete-null-pointer-checks -fdisable-tree-phiopt1" } */ > +/* phiopt1 is disabled so the non-null test reaches VRP as a branch. Early > + phiopt now lowers it to a boolean mask that is folded even earlier, > + which pr21090-2.c covers. */ > > int g, h; > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr68198.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr68198.c > index 125072941da..3b33961268f 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/pr68198.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr68198.c > @@ -1,5 +1,7 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-threadfull1-details -fdisable-tree-ethread" > } */ > +/* { dg-options "-O2 -fdump-tree-threadfull1-details -fdisable-tree-ethread > -fdisable-tree-phiopt1" } */ > +/* phiopt1 is disabled: early phiopt now lowers the remove test to a boolean > + mask and these threading opportunities change shape. */ > > extern void abort (void); > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index e12dc7a8b0c..f4c6622b368 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -640,8 +640,57 @@ factor_out_conditional_operation (edge e0, edge e1, > basic_block merge, > } > > > +/* Return the statement defining VAL when that definition is one of the > + match-generated statements of the not-yet-inserted sequence being > + examined. Such statements have no basic block. Default definitions > + also have no basic block, but are represented by GIMPLE_NOP. > + Return NULL for those and for statements already in the CFG. > + The sequence can also contain statements the simplification ended up > + not using. Walking the use-def chain from the final operation never > + visits those. */ > + > +static gimple * > +phiopt_seq_def_stmt (tree val) > +{ > + if (TREE_CODE (val) != SSA_NAME) > + return NULL; > + gimple *def = SSA_NAME_DEF_STMT (val); > + gcc_checking_assert (def); > + if (gimple_bb (def) || gimple_nop_p (def)) > + return NULL; > + return def; > +} > + > +/* S is a nonnull statement in a comparison-mask chain. If S is a > + conversion whose operand is defined in the generated sequence, return that > + definition. Return NULL for any other conversion and S if S is not a > + conversion. */ > + > +static gimple * > +phiopt_skip_mask_conversion (gimple *s) > +{ > + if (is_gimple_assign (s) > + && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (s))) > + return phiopt_seq_def_stmt (gimple_assign_rhs1 (s)); > + return s; > +} > + > +/* Return TRUE if S is a comparison whose operands are pre-existing values > + (not defined by other statements of the generated sequence), that is, the > + root of a materializable comparison mask chain. */ > + > +static bool > +phiopt_self_contained_cmp_p (gimple *s) > +{ > + return (is_gimple_assign (s) > + && TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison > + && !phiopt_seq_def_stmt (gimple_assign_rhs1 (s)) > + && !phiopt_seq_def_stmt (gimple_assign_rhs2 (s))); > +} > + > /* Return TRUE if SEQ/OP pair should be allowed during early phiopt. > - Currently this is to allow MIN/MAX and ABS/NEGATE and constants. */ > + This allows MIN/MAX, ABS/NEGATE, constants and the comparison-mask > + forms handled below. */ > static bool > phiopt_early_allow (gimple_seq &seq, gimple_match_op &op) > { > @@ -650,8 +699,8 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op) > return false; > tree_code code = (tree_code)op.code; > > - /* For non-empty sequence, only allow one statement > - a MIN/MAX and an original MIN/MAX. */ > + /* For a MIN/MAX with a non-empty sequence, only allow a single > + MIN/MAX assignment. Comparison-mask sequences are handled below. */ > if (!gimple_seq_empty_p (seq)) > { > if (code == MIN_EXPR || code == MAX_EXPR) > @@ -666,6 +715,44 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op) > code = gimple_assign_rhs_code (stmt); > return code == MIN_EXPR || code == MAX_EXPR; > } > + /* Accept the comparison-mask forms CMP, (convert)CMP, -CMP, > + -(convert)CMP, (convert)-CMP and (convert)-(convert)CMP, so a mask > + such as "cmp ? -1 : 0" / "cmp ? 1 : 0" can be materialized as > + -(int)cmp / (int)cmp, exposing its comparison (PR126043). Each > + shape is validated along the use-def chain from the final > + operation. Unused statements in SEQ are ignored. This is only the > + structural test. match_simplify_replacement additionally keeps > + user predictors alive and protects embedded diamonds. */ > + if (TREE_CODE_CLASS (code) == tcc_comparison) > + /* A bare comparison of pre-existing values. Nothing else of SEQ > + is used. */ > + return (!phiopt_seq_def_stmt (op.ops[0]) > + && !phiopt_seq_def_stmt (op.ops[1])); > + if (code == NEGATE_EXPR || CONVERT_EXPR_CODE_P (code)) > + { > + gimple *s = phiopt_seq_def_stmt (op.ops[0]); > + if (!s) > + return false; > + bool saw_negate = (code == NEGATE_EXPR); > + if (!saw_negate && is_gimple_assign (s) > + && gimple_assign_rhs_code (s) == NEGATE_EXPR) > + { > + /* The optional negation under the outer conversion. */ > + saw_negate = true; > + s = phiopt_seq_def_stmt (gimple_assign_rhs1 (s)); > + if (!s) > + return false; > + } > + if (saw_negate) > + { > + /* The optional conversion between the negation and the > + comparison. */ > + s = phiopt_skip_mask_conversion (s); > + if (!s) > + return false; > + } > + return phiopt_self_contained_cmp_p (s); > + } > return false; > } > > @@ -684,7 +771,8 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op) > case FIXED_CST: > return true; > default: > - return false; > + /* A bare comparison (CMP) is also allowed. */ > + return TREE_CODE_CLASS (code) == tcc_comparison; > } > } > > @@ -1101,25 +1189,35 @@ match_simplify_replacement (basic_block cond_bb, > basic_block middle_bb, > } > > /* For early phiopt, we don't want to lose user generated predictors > - if the phiopt is converting `if (a)` into `a` as that might > - be jump threaded later on so we want to keep around the > - predictors. */ > + if the phiopt is converting `if (a)` into `a` (or into the new > + (convert)cmp / -(convert)cmp mask forms) as that might be jump > + threaded later on so we want to keep around the predictors. */ > if (early_p && result && TREE_CODE (result) == SSA_NAME) > { > bool check_it = false; > + bool multi_stmt_mask = false; > tree cmp0 = gimple_cond_lhs (stmt); > tree cmp1 = gimple_cond_rhs (stmt); > if (result == cmp0 || result == cmp1) > check_it = true; > - else if (gimple_seq_singleton_p (seq)) > - { > - gimple *stmt = gimple_seq_first_stmt (seq); > - if (is_gimple_assign (stmt) > - && result == gimple_assign_lhs (stmt) > - && TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) > - == tcc_comparison) > - check_it = true; > - } > + else if (!gimple_seq_empty_p (seq)) > + { > + /* The mask forms phiopt_early_allow accepts end in a comparison, > + conversion or negation defining the result. */ > + gimple *last = gimple_seq_last_stmt (seq); > + if (is_gimple_assign (last) > + && result == gimple_assign_lhs (last)) > + { > + tree_code c = gimple_assign_rhs_code (last); > + if (TREE_CODE_CLASS (c) == tcc_comparison > + || CONVERT_EXPR_CODE_P (c) > + || c == NEGATE_EXPR) > + { > + check_it = true; > + multi_stmt_mask = TREE_CODE_CLASS (c) != tcc_comparison; > + } > + } > + } > if (!check_it) > ; > else if (contains_hot_cold_predict (middle_bb)) > @@ -1128,6 +1226,26 @@ match_simplify_replacement (basic_block cond_bb, > basic_block middle_bb, > && middle_bb != middle_bb_alt > && contains_hot_cold_predict (middle_bb_alt)) > return false; > + /* Materializing a mask for a diamond that other paths enter (through > + its join or through a shared arm) leaves a PHI mixing the mask with > + the other paths' values and a residual sub-diamond. If the generated > + mask has more than one statement, or the condition block has a PHI > + or a statement before its terminating condition, the residual middle > + cannot be moved by late phiopt. Nothing downstream then recovers the > + branchless form, while ifcombine does handle the original nested > + branches, for example into ccmp chains. A bare comparison from a > + condition-only block keeps the residual middle movable. */ > + else if (result != cmp0 && result != cmp1 > + && (multi_stmt_mask > + || !gimple_seq_empty_p (phi_nodes (cond_bb)) > + || last_and_only_stmt (cond_bb) != stmt) > + && (EDGE_COUNT (gimple_bb (phi)->preds) != 2 > + || (middle_bb != gimple_bb (phi) > + && !single_pred_p (middle_bb)) > + || (threeway_p > + && middle_bb_alt != gimple_bb (phi) > + && !single_pred_p (middle_bb_alt)))) > + return false;
Can you show a testcase where this matters? Because I want to remove this check because I think it is NOT needed and even what is written is not true: ``` Nothing downstream then recovers the > + branchless form, while ifcombine does handle the original nested > + branches, for example into ccmp chains. ``` because phiopt will; (match has a pattern for `a ? b : 0`->`a & b` where b is zero_one). Reassociation will also handle this too. If the problem is the factoring of a conversion; then that is a different problem which I have a patch already which is related to the other cases I mentioned. And if this is about trapping floating point comparisons; that is a different problem altogether which ifcombine does not handle currently nor does phiopt. I am working on fixing that for a different patch/reason. Thanks, Andrea > } > > if (!result) > -- > 2.50.1 (Apple Git-155) >
