Message ID | faba6d46-0f9e-f768-e2dc-5b661565c529@suse.cz |
---|---|
State | New |
Headers | show |
On 06/21/2017 03:06 PM, Martin Liška wrote: > Hello. > > There's one additional predictor enhancement that is GOTO predict that > used to working. Following patch adds expect statement for C and C++ family > languages. > > There's one fallout which is vrp24.c test-case, where only 'Simplified relational' > appears just once. Adding Richi and Patrick who can probably help how to fix the > test-case. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? > Martin > And I forgot to mention hitrate on SPEC2017: HEURISTICS BRANCHES (REL) BR. HITRATE HITRATE COVERAGE COVERAGE (REL) predict.def (REL) goto 622 1.0% 64.31% 65.92% / 83.70% 725127790 725.13M 0.1% Which says it's quite rare predictor, but with quite nice hitrate. Martin
On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: > Hello. > > There's one additional predictor enhancement that is GOTO predict that > used to working. Following patch adds expect statement for C and C++ family > languages. > > There's one fallout which is vrp24.c test-case, where only 'Simplified relational' > appears just once. Adding Richi and Patrick who can probably help how to fix the > test-case. Happens to be optimized better now, there's only one predicate to simplify left in the IL input to VRP1. I suggest to change it to match 1 times and add -fdump-tree-optimized to dg-options and /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ to verify we have 3 conditions left. > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? > Martin
On 06/22/2017 12:27 PM, Richard Biener wrote: > On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: >> Hello. >> >> There's one additional predictor enhancement that is GOTO predict that >> used to working. Following patch adds expect statement for C and C++ family >> languages. >> >> There's one fallout which is vrp24.c test-case, where only 'Simplified relational' >> appears just once. Adding Richi and Patrick who can probably help how to fix the >> test-case. > > Happens to be optimized better now, there's only one predicate to simplify > left in the IL input to VRP1. I suggest to change it to match 1 times and add > -fdump-tree-optimized to dg-options and > > /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ > > to verify we have 3 conditions left. Thanks for help. What about the comment: /* The first n_sets > 0 test can be simplfiied into n_sets == 1 since n_sets can only have the values [0, 1] as it's the result of a boolean operation. The second n_sets > 0 test can also be simplified into n_sets == 1 as the only way to reach the tests is when n_sets <= 1 and the only value which satisfies both conditions is n_sets == 1. */ I guess just only one can be valid? Or is it a different story now? Martin > >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? >> Martin
On Thu, Jun 22, 2017 at 12:57 PM, Martin Liška <mliska@suse.cz> wrote: > On 06/22/2017 12:27 PM, Richard Biener wrote: >> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: >>> Hello. >>> >>> There's one additional predictor enhancement that is GOTO predict that >>> used to working. Following patch adds expect statement for C and C++ family >>> languages. >>> >>> There's one fallout which is vrp24.c test-case, where only 'Simplified relational' >>> appears just once. Adding Richi and Patrick who can probably help how to fix the >>> test-case. >> >> Happens to be optimized better now, there's only one predicate to simplify >> left in the IL input to VRP1. I suggest to change it to match 1 times and add >> -fdump-tree-optimized to dg-options and >> >> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ >> >> to verify we have 3 conditions left. > > Thanks for help. > What about the comment: > > /* The first n_sets > 0 test can be simplfiied into n_sets == 1 since > n_sets can only have the values [0, 1] as it's the result of a > boolean operation. > > The second n_sets > 0 test can also be simplified into n_sets == 1 > as the only way to reach the tests is when n_sets <= 1 and the only > value which satisfies both conditions is n_sets == 1. */ > > I guess just only one can be valid? Or is it a different story now? The 2nd one is handled by earlier jump-threading. > Martin > >> >>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>> >>> Ready to be installed? >>> Martin >
PING^1 Can you please Honza give a formal approval for the patch? Thanks, Martin On 06/22/2017 01:47 PM, Richard Biener wrote: > On Thu, Jun 22, 2017 at 12:57 PM, Martin Liška <mliska@suse.cz> wrote: >> On 06/22/2017 12:27 PM, Richard Biener wrote: >>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: >>>> Hello. >>>> >>>> There's one additional predictor enhancement that is GOTO predict that >>>> used to working. Following patch adds expect statement for C and C++ family >>>> languages. >>>> >>>> There's one fallout which is vrp24.c test-case, where only 'Simplified relational' >>>> appears just once. Adding Richi and Patrick who can probably help how to fix the >>>> test-case. >>> >>> Happens to be optimized better now, there's only one predicate to simplify >>> left in the IL input to VRP1. I suggest to change it to match 1 times and add >>> -fdump-tree-optimized to dg-options and >>> >>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ >>> >>> to verify we have 3 conditions left. >> >> Thanks for help. >> What about the comment: >> >> /* The first n_sets > 0 test can be simplfiied into n_sets == 1 since >> n_sets can only have the values [0, 1] as it's the result of a >> boolean operation. >> >> The second n_sets > 0 test can also be simplified into n_sets == 1 >> as the only way to reach the tests is when n_sets <= 1 and the only >> value which satisfies both conditions is n_sets == 1. */ >> >> I guess just only one can be valid? Or is it a different story now? > > The 2nd one is handled by earlier jump-threading. > >> Martin >> >>> >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>> >>>> Ready to be installed? >>>> Martin >>
> PING^1 > > Can you please Honza give a formal approval for the patch? OK, thanks! Honza > > Thanks, > Martin > > On 06/22/2017 01:47 PM, Richard Biener wrote: > > On Thu, Jun 22, 2017 at 12:57 PM, Martin Liška <mliska@suse.cz> wrote: > >> On 06/22/2017 12:27 PM, Richard Biener wrote: > >>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: > >>>> Hello. > >>>> > >>>> There's one additional predictor enhancement that is GOTO predict that > >>>> used to working. Following patch adds expect statement for C and C++ family > >>>> languages. > >>>> > >>>> There's one fallout which is vrp24.c test-case, where only 'Simplified relational' > >>>> appears just once. Adding Richi and Patrick who can probably help how to fix the > >>>> test-case. > >>> > >>> Happens to be optimized better now, there's only one predicate to simplify > >>> left in the IL input to VRP1. I suggest to change it to match 1 times and add > >>> -fdump-tree-optimized to dg-options and > >>> > >>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ > >>> > >>> to verify we have 3 conditions left. > >> > >> Thanks for help. > >> What about the comment: > >> > >> /* The first n_sets > 0 test can be simplfiied into n_sets == 1 since > >> n_sets can only have the values [0, 1] as it's the result of a > >> boolean operation. > >> > >> The second n_sets > 0 test can also be simplified into n_sets == 1 > >> as the only way to reach the tests is when n_sets <= 1 and the only > >> value which satisfies both conditions is n_sets == 1. */ > >> > >> I guess just only one can be valid? Or is it a different story now? > > > > The 2nd one is handled by earlier jump-threading. > > > >> Martin > >> > >>> > >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > >>>> > >>>> Ready to be installed? > >>>> Martin > >>
On 06/22/2017 12:27 PM, Richard Biener wrote: > On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: >> Hello. >> >> There's one additional predictor enhancement that is GOTO predict that >> used to working. Following patch adds expect statement for C and C++ family >> languages. >> >> There's one fallout which is vrp24.c test-case, where only 'Simplified relational' >> appears just once. Adding Richi and Patrick who can probably help how to fix the >> test-case. > > Happens to be optimized better now, there's only one predicate to simplify > left in the IL input to VRP1. I suggest to change it to match 1 times and add > -fdump-tree-optimized to dg-options and > > /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ > > to verify we have 3 conditions left. One small note, I see 4 conditions in optimized dump: sss (struct rtx_def * insn, int code1, int code2, int code3) { int D1544; struct rtx_def * body; _Bool D1562; <bb 2> [100.00%] [count: INV]: body_5 = insn_4(D)->u.fld[5].rt_rtx; D1544_6 = body_5->code; if (D1544_6 == 55) goto <bb 7> (L7); [34.00%] [count: INV] else goto <bb 3>; [66.00%] [count: INV] <bb 3> [66.00%] [count: INV]: if (code3_7(D) == 99) goto <bb 4>; [34.00%] [count: INV] else goto <bb 8> (L16); [66.00%] [count: INV] <bb 4> [22.44%] [count: INV]: D1562_9 = code1_8(D) == 10; if (D1562_9 == 1) goto <bb 7> (L7); [64.00%] [count: INV] else goto <bb 8> (L16); [36.00%] [count: INV] <bb 5> [9.82%] [count: INV]: arf (); <bb 6> [46.68%] [count: INV]: frob (); [tail call] goto <bb 8> (L16); [100.00%] [count: INV] L7 [48.36%] [count: INV]: if (code2_12(D) == 42) goto <bb 5>; [20.24%] [count: INV] else goto <bb 6>; [79.76%] [count: INV] L16 [100.00%] [count: INV]: return; } Is it a problem or adjusting to 4 is fine? Martin > >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? >> Martin
Richi? Thanks On 06/30/2017 03:48 PM, Martin Liška wrote: > On 06/22/2017 12:27 PM, Richard Biener wrote: >> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: >>> Hello. >>> >>> There's one additional predictor enhancement that is GOTO predict that >>> used to working. Following patch adds expect statement for C and C++ family >>> languages. >>> >>> There's one fallout which is vrp24.c test-case, where only 'Simplified relational' >>> appears just once. Adding Richi and Patrick who can probably help how to fix the >>> test-case. >> >> Happens to be optimized better now, there's only one predicate to simplify >> left in the IL input to VRP1. I suggest to change it to match 1 times and add >> -fdump-tree-optimized to dg-options and >> >> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ >> >> to verify we have 3 conditions left. > > One small note, I see 4 conditions in optimized dump: > > sss (struct rtx_def * insn, int code1, int code2, int code3) > { > int D1544; > struct rtx_def * body; > _Bool D1562; > > <bb 2> [100.00%] [count: INV]: > body_5 = insn_4(D)->u.fld[5].rt_rtx; > D1544_6 = body_5->code; > if (D1544_6 == 55) > goto <bb 7> (L7); [34.00%] [count: INV] > else > goto <bb 3>; [66.00%] [count: INV] > > <bb 3> [66.00%] [count: INV]: > if (code3_7(D) == 99) > goto <bb 4>; [34.00%] [count: INV] > else > goto <bb 8> (L16); [66.00%] [count: INV] > > <bb 4> [22.44%] [count: INV]: > D1562_9 = code1_8(D) == 10; > if (D1562_9 == 1) > goto <bb 7> (L7); [64.00%] [count: INV] > else > goto <bb 8> (L16); [36.00%] [count: INV] > > <bb 5> [9.82%] [count: INV]: > arf (); > > <bb 6> [46.68%] [count: INV]: > frob (); [tail call] > goto <bb 8> (L16); [100.00%] [count: INV] > > L7 [48.36%] [count: INV]: > if (code2_12(D) == 42) > goto <bb 5>; [20.24%] [count: INV] > else > goto <bb 6>; [79.76%] [count: INV] > > L16 [100.00%] [count: INV]: > return; > > } > > Is it a problem or adjusting to 4 is fine? > > Martin > >> >>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>> >>> Ready to be installed? >>> Martin >
On Mon, Jul 31, 2017 at 9:46 AM, Martin Liška <mliska@suse.cz> wrote: > Richi? 4 is fine. > Thanks > > On 06/30/2017 03:48 PM, Martin Liška wrote: >> On 06/22/2017 12:27 PM, Richard Biener wrote: >>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška <mliska@suse.cz> wrote: >>>> Hello. >>>> >>>> There's one additional predictor enhancement that is GOTO predict that >>>> used to working. Following patch adds expect statement for C and C++ family >>>> languages. >>>> >>>> There's one fallout which is vrp24.c test-case, where only 'Simplified relational' >>>> appears just once. Adding Richi and Patrick who can probably help how to fix the >>>> test-case. >>> >>> Happens to be optimized better now, there's only one predicate to simplify >>> left in the IL input to VRP1. I suggest to change it to match 1 times and add >>> -fdump-tree-optimized to dg-options and >>> >>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */ >>> >>> to verify we have 3 conditions left. >> >> One small note, I see 4 conditions in optimized dump: >> >> sss (struct rtx_def * insn, int code1, int code2, int code3) >> { >> int D1544; >> struct rtx_def * body; >> _Bool D1562; >> >> <bb 2> [100.00%] [count: INV]: >> body_5 = insn_4(D)->u.fld[5].rt_rtx; >> D1544_6 = body_5->code; >> if (D1544_6 == 55) >> goto <bb 7> (L7); [34.00%] [count: INV] >> else >> goto <bb 3>; [66.00%] [count: INV] >> >> <bb 3> [66.00%] [count: INV]: >> if (code3_7(D) == 99) >> goto <bb 4>; [34.00%] [count: INV] >> else >> goto <bb 8> (L16); [66.00%] [count: INV] >> >> <bb 4> [22.44%] [count: INV]: >> D1562_9 = code1_8(D) == 10; >> if (D1562_9 == 1) >> goto <bb 7> (L7); [64.00%] [count: INV] >> else >> goto <bb 8> (L16); [36.00%] [count: INV] >> >> <bb 5> [9.82%] [count: INV]: >> arf (); >> >> <bb 6> [46.68%] [count: INV]: >> frob (); [tail call] >> goto <bb 8> (L16); [100.00%] [count: INV] >> >> L7 [48.36%] [count: INV]: >> if (code2_12(D) == 42) >> goto <bb 5>; [20.24%] [count: INV] >> else >> goto <bb 6>; [79.76%] [count: INV] >> >> L16 [100.00%] [count: INV]: >> return; >> >> } >> >> Is it a problem or adjusting to 4 is fine? >> >> Martin >> >>> >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>> >>>> Ready to be installed? >>>> Martin >> >
From 4963172e22489a83caa866854386b6d8b5a62f7a Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Mon, 19 Jun 2017 15:44:34 +0200 Subject: [PATCH] Recover GOTO predictor. gcc/c/ChangeLog: 2017-06-21 Jan Hubicka <hubicka@ucw.cz> Martin Liska <mliska@suse.cz> * c-typeck.c (c_finish_goto_label): Build gimple predict stament. gcc/ChangeLog: 2017-06-21 Jan Hubicka <hubicka@ucw.cz> Martin Liska <mliska@suse.cz> * predict.def: Remove old comment and adjust probability. * gimplify.c (should_warn_for_implicit_fallthrough): Ignore PREDICT statements. gcc/testsuite/ChangeLog: 2017-06-21 Jan Hubicka <hubicka@ucw.cz> Martin Liska <mliska@suse.cz> * gcc.dg/predict-15.c: New test. * gcc.dg/tree-ssa/attr-hotcold-2.c: Update the test-case. gcc/cp/ChangeLog: 2017-06-21 Jan Hubicka <hubicka@ucw.cz> Martin Liska <mliska@suse.cz> * pt.c (tsubst_copy): Copy PREDICT_EXPR. * semantics.c (finish_goto_stmt): Build gimple predict stament. * constexpr.c (potential_constant_expression_1): Handle PREDICT_EXPR. --- gcc/c/c-typeck.c | 1 + gcc/cp/constexpr.c | 1 + gcc/cp/pt.c | 2 ++ gcc/cp/semantics.c | 2 ++ gcc/gimplify.c | 4 +++- gcc/predict.def | 5 ++--- gcc/testsuite/gcc.dg/predict-15.c | 17 +++++++++++++++++ gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c | 13 ++++++------- 8 files changed, 34 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/predict-15.c diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 4d067e96dd3..c677c3e9ff5 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -9824,6 +9824,7 @@ c_finish_goto_label (location_t loc, tree label) return NULL_TREE; TREE_USED (decl) = 1; { + add_stmt (build_predict_expr (PRED_GOTO, NOT_TAKEN)); tree t = build1 (GOTO_EXPR, void_type_node, decl); SET_EXPR_LOCATION (t, loc); return add_stmt (t); diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 5a574524866..788e02d7372 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -5784,6 +5784,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, case CLEANUP_STMT: case EMPTY_CLASS_EXPR: + case PREDICT_EXPR: return false; case GOTO_EXPR: diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 69ca9291960..445b46da2de 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -15111,6 +15111,8 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) return tsubst_binary_left_fold (t, args, complain, in_decl); case BINARY_RIGHT_FOLD_EXPR: return tsubst_binary_right_fold (t, args, complain, in_decl); + case PREDICT_EXPR: + return t; default: /* We shouldn't get here, but keep going if !flag_checking. */ diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 5fe772a49e3..a5fcc7b2c63 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see #include "omp-general.h" #include "convert.h" #include "gomp-constants.h" +#include "predict.h" /* There routines provide a modular interface to perform many parsing operations. They may therefore be used during actual parsing, or @@ -630,6 +631,7 @@ finish_goto_stmt (tree destination) check_goto (destination); + add_stmt (build_predict_expr (PRED_GOTO, NOT_TAKEN)); return add_stmt (build_stmt (input_location, GOTO_EXPR, destination)); } diff --git a/gcc/gimplify.c b/gcc/gimplify.c index f45d3bfd13a..0ea8d4ea07b 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -2042,7 +2042,9 @@ should_warn_for_implicit_fallthrough (gimple_stmt_iterator *gsi_p, tree label) gsi = *gsi_p; /* Skip all immediately following labels. */ - while (!gsi_end_p (gsi) && gimple_code (gsi_stmt (gsi)) == GIMPLE_LABEL) + while (!gsi_end_p (gsi) + && (gimple_code (gsi_stmt (gsi)) == GIMPLE_LABEL + || gimple_code (gsi_stmt (gsi)) == GIMPLE_PREDICT)) gsi_next (&gsi); /* { ... something; default:; } */ diff --git a/gcc/predict.def b/gcc/predict.def index f7b2bf7738c..326c39ae4c9 100644 --- a/gcc/predict.def +++ b/gcc/predict.def @@ -132,9 +132,8 @@ DEF_PREDICTOR (PRED_RECURSIVE_CALL, "recursive call", HITRATE (75), 0) DEF_PREDICTOR (PRED_TREE_EARLY_RETURN, "early return (on trees)", HITRATE (66), 0) -/* Branch containing goto is probably not taken. - FIXME: Currently not used. */ -DEF_PREDICTOR (PRED_GOTO, "goto", HITRATE (70), 0) +/* Branch containing goto is probably not taken. */ +DEF_PREDICTOR (PRED_GOTO, "goto", HITRATE (66), 0) /* Branch ending with return constant is probably not taken. */ DEF_PREDICTOR (PRED_CONST_RETURN, "const return", HITRATE (69), 0) diff --git a/gcc/testsuite/gcc.dg/predict-15.c b/gcc/testsuite/gcc.dg/predict-15.c new file mode 100644 index 00000000000..2a8c3ea8597 --- /dev/null +++ b/gcc/testsuite/gcc.dg/predict-15.c @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-profile_estimate" } */ + +int main(int argc, char **argv) +{ + if (argc == 123) + goto exit; + else + { + return 0; + } + +exit: + return 1; +} + +/* { dg-final { scan-tree-dump "goto heuristics of edge" "profile_estimate"} } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c b/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c index 184dd10ddae..d5999e1bf6f 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/attr-hotcold-2.c @@ -1,8 +1,7 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -fdump-tree-profile_estimate-blocks-details" } */ +/* { dg-options "-O2 -fdump-tree-profile_estimate" } */ -void g(void); -void h(void); +int v1, v2; void f(int x, int y) { if (x) goto A; @@ -10,19 +9,19 @@ void f(int x, int y) return; A: __attribute__((cold)) - g(); + v1 = x; return; B: __attribute__((hot)) - h(); + v2 = y; return; } /* { 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 "A \\\[0\\\..*\\\]" "profile_estimate" } } */ +/* { dg-final { scan-tree-dump "combined heuristics: 0\\\..*" "profile_estimate" } } */ /* Note: we're attempting to match some number > 6000, i.e. > 60%. The exact number ought to be tweekable without having to juggle the testcase around too much. */ -/* { dg-final { scan-tree-dump "B \\\[\[6-9\]\[0-9\]\\\..*\\\]" "profile_estimate" } } */ +/* { dg-final { scan-tree-dump "combined heuristics: \[6-9\]\[0-9\]\\\..*" "profile_estimate" } } */ -- 2.13.1