From: Richard Biener Date: Tue, 3 Nov 2020 14:03:41 +0000 (+0100) Subject: tree-optimization/97623 - Avoid PRE hoist insertion iteration X-Git-Tag: releases/gcc-10.3.0~418 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eddcb627ccfbd97e025cf366cc3f3bad76211785;p=thirdparty%2Fgcc.git tree-optimization/97623 - Avoid PRE hoist insertion iteration We are not really interested in PRE opportunities exposed by hoisting but only the other way around. So this moves hoist insertion after PRE iteration finished and removes hoist insertion iteration alltogether. It also guards access to NEW_SETS properly. 2020-11-11 Richard Biener PR tree-optimization/97623 * tree-ssa-pre.c (insert): Move hoist insertion after PRE insertion iteration and do not iterate it. (create_expression_by_pieces): Guard NEW_SETS access. (insert_into_preds_of_block): Likewise. * gcc.dg/tree-ssa/ssa-hoist-3.c: Adjust. * gcc.dg/tree-ssa/ssa-hoist-7.c: Likewise. * gcc.dg/tree-ssa/ssa-pre-30.c: Likewise. --- diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-3.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-3.c index 51ba59c9ab6b..de3051bfb50c 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-3.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-3.c @@ -15,4 +15,4 @@ int test (int a, int b, int c, int g) /* We should hoist and CSE only the multiplication. */ /* { dg-final { scan-tree-dump-times " \\* " 1 "pre" } } */ -/* { dg-final { scan-tree-dump "Insertions: 1" "pre" } } */ +/* { dg-final { scan-tree-dump "HOIST inserted: 1" "pre" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-7.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-7.c index ce9cec61668a..fdb6a3ed3499 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-7.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-hoist-7.c @@ -49,6 +49,6 @@ void foo (int a, int b, int c, int d, int e, int x, int y, int z) /* Now inserting x + y five times is unnecessary but the cascading cannot be avoided with the simple-minded dataflow. But make sure - we do the insertions all in the first iteration. */ -/* { dg-final { scan-tree-dump "insert iterations == 2" "pre" } } */ + we do not iterate PRE insertion. */ +/* { dg-final { scan-tree-dump "insert iterations == 1" "pre" } } */ /* { dg-final { scan-tree-dump "HOIST inserted: 5" "pre" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-30.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-30.c index 59af63ad5aca..cf9317372d64 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-30.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-30.c @@ -24,4 +24,4 @@ bar (int b, int x) /* We should see the partial redundant loads of f even though they are using different types (of the same size). */ -/* { dg-final { scan-tree-dump-times "Replaced MEM" 2 "pre" } } */ +/* { dg-final { scan-tree-dump-times "Replaced MEM" 3 "pre" } } */ diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c index 8776805f5dbb..465f843fdc02 100644 --- a/gcc/tree-ssa-pre.c +++ b/gcc/tree-ssa-pre.c @@ -2953,7 +2953,8 @@ create_expression_by_pieces (basic_block block, pre_expr expr, VN_INFO (forcedname)->value_id = get_next_value_id (); nameexpr = get_or_alloc_expr_for_name (forcedname); add_to_value (VN_INFO (forcedname)->value_id, nameexpr); - bitmap_value_replace_in_set (NEW_SETS (block), nameexpr); + if (NEW_SETS (block)) + bitmap_value_replace_in_set (NEW_SETS (block), nameexpr); bitmap_value_replace_in_set (AVAIL_OUT (block), nameexpr); } @@ -3118,8 +3119,8 @@ insert_into_preds_of_block (basic_block block, unsigned int exprnum, bitmap_insert_into_set (PHI_GEN (block), newphi); bitmap_value_replace_in_set (AVAIL_OUT (block), newphi); - bitmap_insert_into_set (NEW_SETS (block), - newphi); + if (NEW_SETS (block)) + bitmap_insert_into_set (NEW_SETS (block), newphi); /* If we insert a PHI node for a conversion of another PHI node in the same basic-block try to preserve range information. @@ -3645,15 +3646,6 @@ insert (void) fprintf (dump_file, "Starting insert iteration %d\n", num_iterations); changed = false; - /* Insert expressions for hoisting. Do a backward walk here since - inserting into BLOCK exposes new opportunities in its predecessors. */ - if (flag_code_hoisting) - for (int idx = rpo_num - 1; idx >= 0; --idx) - { - basic_block block = BASIC_BLOCK_FOR_FN (cfun, rpo[idx]); - if (EDGE_COUNT (block->succs) >= 2) - changed |= do_hoist_insertion (block); - } for (int idx = 0; idx < rpo_num; ++idx) { basic_block block = BASIC_BLOCK_FOR_FN (cfun, rpo[idx]); @@ -3701,6 +3693,28 @@ insert (void) statistics_histogram_event (cfun, "insert iterations", num_iterations); + /* AVAIL_OUT is not needed after insertion so we don't have to + propagate NEW_SETS from hoist insertion. */ + FOR_ALL_BB_FN (bb, cfun) + { + bitmap_set_pool.remove (NEW_SETS (bb)); + NEW_SETS (bb) = NULL; + } + + /* Insert expressions for hoisting. Do a backward walk here since + inserting into BLOCK exposes new opportunities in its predecessors. + Since PRE and hoist insertions can cause back-to-back iteration + and we are interested in PRE insertion exposed hoisting opportunities + but not in hoisting exposed PRE ones do hoist insertion only after + PRE insertion iteration finished and do not iterate it. */ + if (flag_code_hoisting) + for (int idx = rpo_num - 1; idx >= 0; --idx) + { + basic_block block = BASIC_BLOCK_FOR_FN (cfun, rpo[idx]); + if (EDGE_COUNT (block->succs) >= 2) + changed |= do_hoist_insertion (block); + } + free (rpo); }