> On Aug 28, 2017, at 7:37 AM, Richard Biener <richard.guent...@gmail.com> > wrote: > > Not sure, but would it be fixed in a similar way when writing <snip> > ?
Thanks, Richard, that works very well. I decided this was a good opportunity to also simplify the control flow a little with early returns. Here's the result. Bootstrapped and tested on powerpc64le-linux-gnu with no regressions. Is this ok for trunk, and eventually for backport to gcc 5, 6, and 7? (I can omit the control flow cleanups for the older releases if desired.) Thanks, Bill [gcc] 2017-08-03 Bill Schmidt <wschm...@linux.vnet.ibm.com> Jakub Jelinek <ja...@redhat.com> Richard Biener <rguent...@suse.de> PR tree-optimization/81503 * gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure folded constant fits in the target type; reorder tests for clarity. [gcc/testsuite] 2017-08-03 Bill Schmidt <wschm...@linux.vnet.ibm.com> Jakub Jelinek <ja...@redhat.com> Richard Biener <rguent...@suse.de> PR tree-optimization/81503 * gcc.c-torture/execute/pr81503.c: New file. Index: gcc/gimple-ssa-strength-reduction.c =================================================================== --- gcc/gimple-ssa-strength-reduction.c (revision 251369) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -2089,104 +2089,104 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt)); enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt); - /* It is highly unlikely, but possible, that the resulting - bump doesn't fit in a HWI. Abandon the replacement - in this case. This does not affect siblings or dependents - of C. Restriction to signed HWI is conservative for unsigned - types but allows for safe negation without twisted logic. */ - if (wi::fits_shwi_p (bump) - && bump.to_shwi () != HOST_WIDE_INT_MIN - /* It is not useful to replace casts, copies, negates, or adds of - an SSA name and a constant. */ - && cand_code != SSA_NAME - && !CONVERT_EXPR_CODE_P (cand_code) - && cand_code != PLUS_EXPR - && cand_code != POINTER_PLUS_EXPR - && cand_code != MINUS_EXPR - && cand_code != NEGATE_EXPR) + /* It is not useful to replace casts, copies, negates, or adds of + an SSA name and a constant. */ + if (cand_code == SSA_NAME + || CONVERT_EXPR_CODE_P (cand_code) + || cand_code == PLUS_EXPR + || cand_code == POINTER_PLUS_EXPR + || cand_code == MINUS_EXPR + || cand_code == NEGATE_EXPR) + return; + + enum tree_code code = PLUS_EXPR; + tree bump_tree; + gimple *stmt_to_print = NULL; + + if (wi::neg_p (bump)) { - enum tree_code code = PLUS_EXPR; - tree bump_tree; - gimple *stmt_to_print = NULL; + code = MINUS_EXPR; + bump = -bump; + } - /* If the basis name and the candidate's LHS have incompatible - types, introduce a cast. */ - if (!useless_type_conversion_p (target_type, TREE_TYPE (basis_name))) - basis_name = introduce_cast_before_cand (c, target_type, basis_name); - if (wi::neg_p (bump)) + /* It is possible that the resulting bump doesn't fit in target_type. + Abandon the replacement in this case. This does not affect + siblings or dependents of C. */ + if (bump != wi::ext (bump, TYPE_PRECISION (target_type), + TYPE_SIGN (target_type))) + return; + + bump_tree = wide_int_to_tree (target_type, bump); + + /* If the basis name and the candidate's LHS have incompatible types, + introduce a cast. */ + if (!useless_type_conversion_p (target_type, TREE_TYPE (basis_name))) + basis_name = introduce_cast_before_cand (c, target_type, basis_name); + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fputs ("Replacing: ", dump_file); + print_gimple_stmt (dump_file, c->cand_stmt, 0); + } + + if (bump == 0) + { + tree lhs = gimple_assign_lhs (c->cand_stmt); + gassign *copy_stmt = gimple_build_assign (lhs, basis_name); + gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); + slsr_cand_t cc = c; + gimple_set_location (copy_stmt, gimple_location (c->cand_stmt)); + gsi_replace (&gsi, copy_stmt, false); + c->cand_stmt = copy_stmt; + while (cc->next_interp) { - code = MINUS_EXPR; - bump = -bump; + cc = lookup_cand (cc->next_interp); + cc->cand_stmt = copy_stmt; } - - bump_tree = wide_int_to_tree (target_type, bump); - if (dump_file && (dump_flags & TDF_DETAILS)) + stmt_to_print = copy_stmt; + } + else + { + tree rhs1, rhs2; + if (cand_code != NEGATE_EXPR) { + rhs1 = gimple_assign_rhs1 (c->cand_stmt); + rhs2 = gimple_assign_rhs2 (c->cand_stmt); + } + if (cand_code != NEGATE_EXPR + && ((operand_equal_p (rhs1, basis_name, 0) + && operand_equal_p (rhs2, bump_tree, 0)) + || (operand_equal_p (rhs1, bump_tree, 0) + && operand_equal_p (rhs2, basis_name, 0)))) { - fputs ("Replacing: ", dump_file); - print_gimple_stmt (dump_file, c->cand_stmt, 0); + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fputs ("(duplicate, not actually replacing)", dump_file); + stmt_to_print = c->cand_stmt; + } } - - if (bump == 0) + else { - tree lhs = gimple_assign_lhs (c->cand_stmt); - gassign *copy_stmt = gimple_build_assign (lhs, basis_name); gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); slsr_cand_t cc = c; - gimple_set_location (copy_stmt, gimple_location (c->cand_stmt)); - gsi_replace (&gsi, copy_stmt, false); - c->cand_stmt = copy_stmt; + gimple_assign_set_rhs_with_ops (&gsi, code, basis_name, bump_tree); + update_stmt (gsi_stmt (gsi)); + c->cand_stmt = gsi_stmt (gsi); while (cc->next_interp) { cc = lookup_cand (cc->next_interp); - cc->cand_stmt = copy_stmt; + cc->cand_stmt = gsi_stmt (gsi); } if (dump_file && (dump_flags & TDF_DETAILS)) - stmt_to_print = copy_stmt; + stmt_to_print = gsi_stmt (gsi); } - else - { - tree rhs1, rhs2; - if (cand_code != NEGATE_EXPR) { - rhs1 = gimple_assign_rhs1 (c->cand_stmt); - rhs2 = gimple_assign_rhs2 (c->cand_stmt); - } - if (cand_code != NEGATE_EXPR - && ((operand_equal_p (rhs1, basis_name, 0) - && operand_equal_p (rhs2, bump_tree, 0)) - || (operand_equal_p (rhs1, bump_tree, 0) - && operand_equal_p (rhs2, basis_name, 0)))) - { - if (dump_file && (dump_flags & TDF_DETAILS)) - { - fputs ("(duplicate, not actually replacing)", dump_file); - stmt_to_print = c->cand_stmt; - } - } - else - { - gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); - slsr_cand_t cc = c; - gimple_assign_set_rhs_with_ops (&gsi, code, - basis_name, bump_tree); - update_stmt (gsi_stmt (gsi)); - c->cand_stmt = gsi_stmt (gsi); - while (cc->next_interp) - { - cc = lookup_cand (cc->next_interp); - cc->cand_stmt = gsi_stmt (gsi); - } - if (dump_file && (dump_flags & TDF_DETAILS)) - stmt_to_print = gsi_stmt (gsi); - } - } + } - if (dump_file && (dump_flags & TDF_DETAILS)) - { - fputs ("With: ", dump_file); - print_gimple_stmt (dump_file, stmt_to_print, 0); - fputs ("\n", dump_file); - } + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fputs ("With: ", dump_file); + print_gimple_stmt (dump_file, stmt_to_print, 0); + fputs ("\n", dump_file); } } Index: gcc/testsuite/gcc.c-torture/execute/pr81503.c =================================================================== --- gcc/testsuite/gcc.c-torture/execute/pr81503.c (nonexistent) +++ gcc/testsuite/gcc.c-torture/execute/pr81503.c (working copy) @@ -0,0 +1,15 @@ +unsigned short a = 41461; +unsigned short b = 3419; +int c = 0; + +void foo() { + if (a + b * ~(0 != 5)) + c = -~(b * ~(0 != 5)) + 2147483647; +} + +int main() { + foo(); + if (c != 2147476810) + return -1; + return 0; +}