Message ID | 569DB2F9.30506@linaro.org |
---|---|
State | New |
Headers | show |
On 01/18/2016 08:52 PM, Kugan wrote: > Hi, > > This is an updated version of > https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02196.html. > > Patch to fix PR66726 missed optimization, factor conversion out of > COND_EXPR caused a regression for targets with branch cost greater than > i.e., testcase gcc.dg/pr46309.c failed for these targets. I posted a > patch for this which had some issues. Please find an updated version of > this patch that now passes regression. > > This patch makes optimize_range_tests understand the factored out > COND_EXPR. i.e., Updated the final_range_test_p to look for the new > pattern. Changed the maybe_optimize_range_tests (which does the inter > basic block range test optimization) accordingly. > > Bootstrapped and regression tested on x86_64-none-linux-gnu with no new > regressions. And also regression tested on arm-none-linux-gnu and > aarch64-none-linux-gnu with no new regressions. > Is this Ok for trunk? > > Thanks, > Kugan > > > gcc/ChangeLog: > > 2016-01-19 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR middle-end/66726 > * tree-ssa-reassoc.c (optimize_range_tests): Handle tcc_compare stmt > whose result is used in PHI. > (maybe_optimize_range_tests): Likewise. > (final_range_test_p): Lokweise. > s/Lokeweise/Likewise/ in the ChangeLog entry. Otherwise this looks OK for the trunk. It really hasn't changed much since the version from July. And while the PR is not marked as such, this is a code quality regression fix for targets with a BRANCH_COST > 1. Thanks for your patience and not letting this get lost. Jeff
On 2016.02.08 at 09:49 -0700, Jeff Law wrote: > On 01/18/2016 08:52 PM, Kugan wrote: > > > >2016-01-19 Kugan Vivekanandarajah <kuganv@linaro.org> > > > > PR middle-end/66726 > > * tree-ssa-reassoc.c (optimize_range_tests): Handle tcc_compare stmt > > whose result is used in PHI. > > (maybe_optimize_range_tests): Likewise. > > (final_range_test_p): Lokweise. > > > Otherwise this looks OK for the trunk. It really hasn't changed much since > the version from July. And while the PR is not marked as such, this is a > code quality regression fix for targets with a BRANCH_COST > 1. This causes LTO/PGO bootstrap on ppc64le to ICE all over the place: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781
On 12/02/16 17:18, Markus Trippelsdorf wrote: > On 2016.02.08 at 09:49 -0700, Jeff Law wrote: >> On 01/18/2016 08:52 PM, Kugan wrote: >>> >>> 2016-01-19 Kugan Vivekanandarajah <kuganv@linaro.org> >>> >>> PR middle-end/66726 >>> * tree-ssa-reassoc.c (optimize_range_tests): Handle tcc_compare stmt >>> whose result is used in PHI. >>> (maybe_optimize_range_tests): Likewise. >>> (final_range_test_p): Lokweise. >>> >> Otherwise this looks OK for the trunk. It really hasn't changed much since >> the version from July. And while the PR is not marked as such, this is a >> code quality regression fix for targets with a BRANCH_COST > 1. > > This causes LTO/PGO bootstrap on ppc64le to ICE all over the place: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781 > Sorry for the breakage. I will revert the patch while I investigate this. Thanks, Kugan
> This causes LTO/PGO bootstrap on ppc64le to ICE all over the place: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781 And a comparison failure with -enable-checking=yes,rtl on x86-64/Linux: Comparing stages 2 and 3 warning: gcc/cc1plus-checksum.o differs warning: gcc/cc1-checksum.o differs Bootstrap comparison failure! gcc/cp/typeck2.o differs gcc/cp/decl.o differs gcc/cp/mangle.o differs gcc/sched-deps.o differs gcc/cselib.o differs gcc/cfgexpand.o differs gcc/ipa-icf-gimple.o differs gcc/gcse.o differs gcc/tree-ssa-alias.o differs gcc/postreload-gcse.o differs gcc/reginfo.o differs gcc/sel-sched.o differs gcc/build/genmatch.o differs gcc/build/genrecog.o differs gcc/gimplify.o differs gcc/tree-inline.o differs gcc/optabs.o differs gcc/emit-rtl.o differs gcc/dwarf2out.o differs gcc/tree-cfg.o differs gcc/tree-eh.o differs gcc/tree-predcom.o differs gcc/tree-chrec.o differs gcc/opts-common.o differs gcc/tree-ssa-phiopt.o differs gcc/asan.o differs gcc/var-tracking.o differs gcc/ipa-prop.o differs gcc/diagnostic-show-locus.o differs gcc/predict.o differs gcc/tree-vectorizer.o differs gcc/auto-profile.o differs gcc/libgcov-driver-tool.o differs gcc/tree-chkp-opt.o differs gcc/gcc.o differs gcc/store-motion.o differs gcc/sched-rgn.o differs gcc/haifa-sched.o differs gcc/ada/rtsfind.o differs gcc/ada/ali.o differs gcc/ada/adaint.o differs gcc/ada/fname-uf.o differs gcc/ada/set_targ.o differs gcc/ipa-icf.o differs gcc/builtins.o differs gcc/fold-const.o differs gcc/reload.o differs gcc/tree-ssa-loop-ivopts.o differs gcc/tree-sra.o differs gcc/tree-dfa.o differs gcc/gimple-ssa-strength-reduction.o differs gcc/tree-ssa-address.o differs gcc/gimple-ssa-backprop.o differs gcc/postreload.o differs gcc/wide-int.o differs gcc/tree-switch-conversion.o differs gcc/real.o differs gcc/tree-ssa-loop-niter.o differs gcc/gimple-match.o differs gcc/simplify-rtx.o differs gcc/tree-vrp.o differs gcc/tree-if-conv.o differs gcc/coverage.o differs gcc/gimple-fold.o differs gcc/function.o differs gcc/i386.o differs gcc/tree-affine.o differs gcc/tree-vect-loop.o differs gcc/tree-ssa-structalias.o differs gcc/c/c-parser.o differs gcc/cfgrtl.o differs gcc/tree-ssa-loop-prefetch.o differs gcc/generic-match.o differs gcc/rtl-chkp.o differs gcc/tree-ssa-coalesce.o differs gcc/mcf.o differs gcc/optabs-libfuncs.o differs libbacktrace/dwarf.o differs libbacktrace/.libs/dwarf.o differs libcpp/lex.o differs Makefile:21544: recipe for target 'compare' failed
On Thu, Feb 11, 2016 at 10:18 PM, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > On 2016.02.08 at 09:49 -0700, Jeff Law wrote: >> On 01/18/2016 08:52 PM, Kugan wrote: >> > >> >2016-01-19 Kugan Vivekanandarajah <kuganv@linaro.org> >> > >> > PR middle-end/66726 >> > * tree-ssa-reassoc.c (optimize_range_tests): Handle tcc_compare stmt >> > whose result is used in PHI. >> > (maybe_optimize_range_tests): Likewise. >> > (final_range_test_p): Lokweise. >> > >> Otherwise this looks OK for the trunk. It really hasn't changed much since >> the version from July. And while the PR is not marked as such, this is a >> code quality regression fix for targets with a BRANCH_COST > 1. > > This causes LTO/PGO bootstrap on ppc64le to ICE all over the place: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69781 > On ia32, it also caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69786 and FAIL: libgomp.oacc-fortran/collapse-4.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -O1 execution test FAIL: libgomp.oacc-fortran/collapse-5.f90 -DACC_DEVICE_TYPE_host=1 -DACC_MEM_SHARED=1 -O1 execution test which were compiled into infinite loop.
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c index e53cc56..d0a5cee 100644 --- a/gcc/tree-ssa-reassoc.c +++ b/gcc/tree-ssa-reassoc.c @@ -2687,18 +2687,33 @@ optimize_range_tests (enum tree_code opcode, # _345 = PHI <_123(N), 1(...), 1(...)> where _234 has bool type, _123 has single use and bb N has a single successor M. This is commonly used in + the last block of a range test. + + Also Return true if STMT is tcc_compare like: + <bb N>: + ... + _234 = a_2(D) == 2; + + <bb M>: + # _345 = PHI <_234(N), 1(...), 1(...)> + _346 = (int) _345; + where _234 has booltype, single use and + bb N has a single successor M. This is commonly used in the last block of a range test. */ static bool final_range_test_p (gimple *stmt) { - basic_block bb, rhs_bb; + basic_block bb, rhs_bb, lhs_bb; edge e; tree lhs, rhs; use_operand_p use_p; gimple *use_stmt; - if (!gimple_assign_cast_p (stmt)) + if (!gimple_assign_cast_p (stmt) + && (!is_gimple_assign (stmt) + || (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) + != tcc_comparison))) return false; bb = gimple_bb (stmt); if (!single_succ_p (bb)) @@ -2709,11 +2724,16 @@ final_range_test_p (gimple *stmt) lhs = gimple_assign_lhs (stmt); rhs = gimple_assign_rhs1 (stmt); - if (!INTEGRAL_TYPE_P (TREE_TYPE (lhs)) - || TREE_CODE (rhs) != SSA_NAME - || TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE) + if (gimple_assign_cast_p (stmt) + && (!INTEGRAL_TYPE_P (TREE_TYPE (lhs)) + || TREE_CODE (rhs) != SSA_NAME + || TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE)) return false; + if (!gimple_assign_cast_p (stmt) + && (TREE_CODE (TREE_TYPE (lhs)) != BOOLEAN_TYPE)) + return false; + /* Test whether lhs is consumed only by a PHI in the only successor bb. */ if (!single_imm_use (lhs, &use_p, &use_stmt)) return false; @@ -2723,10 +2743,20 @@ final_range_test_p (gimple *stmt) return false; /* And that the rhs is defined in the same loop. */ - rhs_bb = gimple_bb (SSA_NAME_DEF_STMT (rhs)); - if (rhs_bb == NULL - || !flow_bb_inside_loop_p (loop_containing_stmt (stmt), rhs_bb)) - return false; + if (gimple_assign_cast_p (stmt)) + { + if (TREE_CODE (rhs) != SSA_NAME + || !(rhs_bb = gimple_bb (SSA_NAME_DEF_STMT (rhs))) + || !flow_bb_inside_loop_p (loop_containing_stmt (stmt), rhs_bb)) + return false; + } + else + { + if (TREE_CODE (lhs) != SSA_NAME + || !(lhs_bb = gimple_bb (SSA_NAME_DEF_STMT (lhs))) + || !flow_bb_inside_loop_p (loop_containing_stmt (stmt), lhs_bb)) + return false; + } return true; } @@ -3119,6 +3149,8 @@ maybe_optimize_range_tests (gimple *stmt) /* stmt is _123 = (int) _234; + OR + _234 = a_2(D) == 2; followed by: <bb M>: @@ -3148,6 +3180,8 @@ maybe_optimize_range_tests (gimple *stmt) of the bitwise or resp. and, recursively. */ if (!get_ops (rhs, code, &ops, loop_containing_stmt (stmt)) + && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) + != tcc_comparison) && has_single_use (rhs)) { /* Otherwise, push the _234 range test itself. */ @@ -3160,6 +3194,23 @@ maybe_optimize_range_tests (gimple *stmt) ops.safe_push (oe); bb_ent.last_idx++; } + else if (!get_ops (lhs, code, &ops, + loop_containing_stmt (stmt)) + && TREE_CODE (lhs) == SSA_NAME + && INTEGRAL_TYPE_P (TREE_TYPE (lhs)) + && is_gimple_assign (stmt) + && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) + == tcc_comparison) + && has_single_use (lhs)) + { + operand_entry *oe = operand_entry_pool.allocate (); + oe->op = lhs; + oe->rank = code; + oe->id = 0; + oe->count = 1; + ops.safe_push (oe); + bb_ent.last_idx++; + } else bb_ent.last_idx = ops.length (); bb_ent.op = rhs; @@ -3243,26 +3294,60 @@ maybe_optimize_range_tests (gimple *stmt) { imm_use_iterator iter; use_operand_p use_p; - gimple *use_stmt, *cast_stmt = NULL; + gimple *use_stmt, *cast_or_tcc_cmp_stmt = NULL; FOR_EACH_IMM_USE_STMT (use_stmt, iter, bbinfo[idx].op) - if (is_gimple_debug (use_stmt)) + if (is_gimple_debug (use_stmt) + || (TREE_CODE (new_op) == SSA_NAME + && !reassoc_stmt_dominates_stmt_p + (SSA_NAME_DEF_STMT (new_op), use_stmt))) continue; - else if (gimple_code (use_stmt) == GIMPLE_COND - || gimple_code (use_stmt) == GIMPLE_PHI) - FOR_EACH_IMM_USE_ON_STMT (use_p, iter) - SET_USE (use_p, new_op); + else if (gimple_code (use_stmt) == GIMPLE_PHI) + FOR_EACH_IMM_USE_ON_STMT (use_p, iter) + SET_USE (use_p, new_op); + else if (gimple_code (use_stmt) == GIMPLE_COND) + { + tree new_type, new_lhs; + gassign *g; + gcond *cond_stmt = as_a <gcond *> (use_stmt); + new_type = TREE_TYPE (gimple_cond_lhs (cond_stmt)); + if (!types_compatible_p (new_type, TREE_TYPE (new_op))) + { + new_lhs = make_ssa_name (new_type); + if (is_gimple_min_invariant (new_op)) + { + new_op = fold_convert (new_type, new_op); + g = gimple_build_assign (new_lhs, new_op); + } + else + g = gimple_build_assign (new_lhs, + CONVERT_EXPR, new_op); + gimple_stmt_iterator gsi = gsi_for_stmt (use_stmt); + gimple_set_uid (g, gimple_uid (use_stmt)); + gimple_set_visited (g, true); + gsi_insert_before (&gsi, g, GSI_SAME_STMT); + } + else + new_lhs = new_op; + FOR_EACH_IMM_USE_ON_STMT (use_p, iter) + SET_USE (use_p, new_lhs); + } + else if ((is_gimple_assign (use_stmt) + && (TREE_CODE_CLASS (gimple_assign_rhs_code (use_stmt)) == tcc_comparison))) + { + cast_or_tcc_cmp_stmt = use_stmt; + } else if (gimple_assign_cast_p (use_stmt)) - cast_stmt = use_stmt; - else - gcc_unreachable (); - if (cast_stmt) + cast_or_tcc_cmp_stmt = use_stmt; + + if (cast_or_tcc_cmp_stmt) { gcc_assert (bb == last_bb); - tree lhs = gimple_assign_lhs (cast_stmt); + tree lhs = gimple_assign_lhs (cast_or_tcc_cmp_stmt); tree new_lhs = make_ssa_name (TREE_TYPE (lhs)); enum tree_code rhs_code - = gimple_assign_rhs_code (cast_stmt); + = gimple_assign_cast_p (cast_or_tcc_cmp_stmt) ? + gimple_assign_rhs_code (cast_or_tcc_cmp_stmt) : CONVERT_EXPR; gassign *g; if (is_gimple_min_invariant (new_op)) { @@ -3271,13 +3356,14 @@ maybe_optimize_range_tests (gimple *stmt) } else g = gimple_build_assign (new_lhs, rhs_code, new_op); - gimple_stmt_iterator gsi = gsi_for_stmt (cast_stmt); - gimple_set_uid (g, gimple_uid (cast_stmt)); + gimple_stmt_iterator gsi = gsi_for_stmt (cast_or_tcc_cmp_stmt); + gimple_set_uid (g, gimple_uid (cast_or_tcc_cmp_stmt)); gimple_set_visited (g, true); - gsi_insert_before (&gsi, g, GSI_SAME_STMT); + gsi_insert_after (&gsi, g, GSI_SAME_STMT); FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs) if (is_gimple_debug (use_stmt)) continue; + else if (is_gimple_assign (use_stmt)); else if (gimple_code (use_stmt) == GIMPLE_COND || gimple_code (use_stmt) == GIMPLE_PHI) FOR_EACH_IMM_USE_ON_STMT (use_p, iter)