public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc r14-4719] tree-ssa-math-opts: Fix up match_uaddc_usubc [PR111845] Date: Wed, 18 Oct 2023 10:38:42 +0000 (GMT) [thread overview] Message-ID: <20231018103842.AB68C3858415@sourceware.org> (raw) https://gcc.gnu.org/g:f1744dd50bb1661c98b694ff907cb0a1be4f6134 commit r14-4719-gf1744dd50bb1661c98b694ff907cb0a1be4f6134 Author: Jakub Jelinek <jakub@redhat.com> Date: Wed Oct 18 12:37:40 2023 +0200 tree-ssa-math-opts: Fix up match_uaddc_usubc [PR111845] GCC ICEs on the first testcase. Successful match_uaddc_usubc ends up with some dead stmts which DCE will remove (hopefully) later all. The ICE is because one of the dead stmts refers to a freed SSA_NAME. The code already gsi_removes a couple of stmts in the /* Remove some statements which can't be kept in the IL because they use SSA_NAME whose setter is going to be removed too. */ section for the same reason (the reason for the freed SSA_NAMEs is that we don't really have a replacement for those cases - all we have after a match is combined overflow from the addition/subtraction of 2 operands + a [0, 1] carry in, but not the individual overflows from the former 2 additions), but for the last (most significant) limb case, where we try to match x = op1 + op2 + carry1 + carry2; or x = op1 - op2 - carry1 - carry2; we just gsi_replace the final stmt, but left around the 2 temporary stmts as dead; if we were unlucky enough that those referenced the carry flag that went away, it ICEs. So, the following patch remembers those temporary statements (rather than trying to rediscover them more expensively) and removes them before the final one is replaced. While working on it, I've noticed we didn't support all the reassociated possibilities of writing the addition of 4 operands or subtracting 3 operands from one, we supported e.g. x = ((op1 + op2) + op3) + op4; x = op1 + ((op2 + op3) + op4); but not x = (op1 + (op2 + op3)) + op4; x = op1 + (op2 + (op3 + op4)); Fixed by the change to inspect also rhs[2] when rhs[1] didn't yield what we were searching for (if non-NULL) - rhs[0] is inspected in the first loop and has different handling for the MINUS_EXPR case. 2023-10-18 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/111845 * tree-ssa-math-opts.cc (match_uaddc_usubc): Remember temporary statements for the 4 operand addition or subtraction of 3 operands from 1 operand cases and remove them when successful. Look for nested additions even from rhs[2], not just rhs[1]. * gcc.dg/pr111845.c: New test. * gcc.target/i386/pr111845.c: New test. Diff: --- gcc/testsuite/gcc.dg/pr111845.c | 16 +++++++++++ gcc/testsuite/gcc.target/i386/pr111845.c | 47 +++++++++++++++++++++++++++++++ gcc/tree-ssa-math-opts.cc | 48 +++++++++++++++++++++----------- 3 files changed, 94 insertions(+), 17 deletions(-) diff --git a/gcc/testsuite/gcc.dg/pr111845.c b/gcc/testsuite/gcc.dg/pr111845.c new file mode 100644 index 000000000000..1bcb4f88e6f1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr111845.c @@ -0,0 +1,16 @@ +/* PR tree-optimization/111845 */ +/* { dg-do compile } */ +/* { dg-options "-O2 --param tree-reassoc-width=2" } */ + +int a, b; +unsigned int c, d, e; + +void +foo (int x) +{ + b += d; + c += b < d; + b += e = a < x; + c += b; + c += b < e; +} diff --git a/gcc/testsuite/gcc.target/i386/pr111845.c b/gcc/testsuite/gcc.target/i386/pr111845.c new file mode 100644 index 000000000000..d52110a40422 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr111845.c @@ -0,0 +1,47 @@ +/* PR tree-optimization/111845 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -g -masm=att" } */ +/* { dg-final { scan-assembler-times "\tadcq\t" 8 { target lp64 } } } */ +/* { dg-final { scan-assembler-times "\tadcl\t" 8 { target ia32 } } } */ + +unsigned long l, m; + +__attribute__((noipa)) void +foo (unsigned long x, unsigned long y, unsigned long h, unsigned long i, int a, int b) +{ + unsigned long c, d; + unsigned long e = __builtin_add_overflow (x, y, &c); + unsigned long f = __builtin_add_overflow (c, a < b, &d); + m = ((h + i) + e) + f; + l = d; +} + +__attribute__((noipa)) void +bar (unsigned long x, unsigned long y, unsigned long h, unsigned long i, int a, int b) +{ + unsigned long c, d; + unsigned long e = __builtin_add_overflow (x, y, &c); + unsigned long f = __builtin_add_overflow (c, a < b, &d); + m = (h + (i + e)) + f; + l = d; +} + +__attribute__((noipa)) void +baz (unsigned long x, unsigned long y, unsigned long h, unsigned long i, int a, int b) +{ + unsigned long c, d; + unsigned long e = __builtin_add_overflow (x, y, &c); + unsigned long f = __builtin_add_overflow (c, a < b, &d); + m = h + (i + (e + f)); + l = d; +} + +__attribute__((noipa)) void +qux (unsigned long x, unsigned long y, unsigned long h, unsigned long i, int a, int b) +{ + unsigned long c, d; + unsigned long e = __builtin_add_overflow (x, y, &c); + unsigned long f = __builtin_add_overflow (c, a < b, &d); + m = h + ((i + e) + f); + l = d; +} diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc index 51c14d6bad9f..363f31646691 100644 --- a/gcc/tree-ssa-math-opts.cc +++ b/gcc/tree-ssa-math-opts.cc @@ -4581,6 +4581,7 @@ match_uaddc_usubc (gimple_stmt_iterator *gsi, gimple *stmt, tree_code code) if (!INTEGRAL_TYPE_P (type) || !TYPE_UNSIGNED (type)) return false; + auto_vec<gimple *, 2> temp_stmts; if (code != BIT_IOR_EXPR && code != BIT_XOR_EXPR) { /* If overflow flag is ignored on the MSB limb, we can end up with @@ -4615,26 +4616,29 @@ match_uaddc_usubc (gimple_stmt_iterator *gsi, gimple *stmt, tree_code code) rhs[0] = gimple_assign_rhs1 (g); tree &r = rhs[2] ? rhs[3] : rhs[2]; r = r2; + temp_stmts.quick_push (g); } else break; } - while (TREE_CODE (rhs[1]) == SSA_NAME && !rhs[3]) - { - gimple *g = SSA_NAME_DEF_STMT (rhs[1]); - if (has_single_use (rhs[1]) - && is_gimple_assign (g) - && gimple_assign_rhs_code (g) == PLUS_EXPR) - { - rhs[1] = gimple_assign_rhs1 (g); - if (rhs[2]) - rhs[3] = gimple_assign_rhs2 (g); - else - rhs[2] = gimple_assign_rhs2 (g); - } - else - break; - } + for (int i = 1; i <= 2; ++i) + while (rhs[i] && TREE_CODE (rhs[i]) == SSA_NAME && !rhs[3]) + { + gimple *g = SSA_NAME_DEF_STMT (rhs[i]); + if (has_single_use (rhs[i]) + && is_gimple_assign (g) + && gimple_assign_rhs_code (g) == PLUS_EXPR) + { + rhs[i] = gimple_assign_rhs1 (g); + if (rhs[2]) + rhs[3] = gimple_assign_rhs2 (g); + else + rhs[2] = gimple_assign_rhs2 (g); + temp_stmts.quick_push (g); + } + else + break; + } /* If there are just 3 addends or one minuend and two subtrahends, check for UADDC or USUBC being pattern recognized earlier. Say r = op1 + op2 + ovf1 + ovf2; where the (ovf1 + ovf2) part @@ -5039,7 +5043,17 @@ match_uaddc_usubc (gimple_stmt_iterator *gsi, gimple *stmt, tree_code code) g = gimple_build_assign (ilhs, IMAGPART_EXPR, build1 (IMAGPART_EXPR, TREE_TYPE (ilhs), nlhs)); if (rhs[2]) - gsi_insert_before (gsi, g, GSI_SAME_STMT); + { + gsi_insert_before (gsi, g, GSI_SAME_STMT); + /* Remove some further statements which can't be kept in the IL because + they can use SSA_NAMEs whose setter is going to be removed too. */ + while (temp_stmts.length ()) + { + g = temp_stmts.pop (); + gsi2 = gsi_for_stmt (g); + gsi_remove (&gsi2, true); + } + } else gsi_replace (gsi, g, true); /* Remove some statements which can't be kept in the IL because they
reply other threads:[~2023-10-18 10:38 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231018103842.AB68C3858415@sourceware.org \ --to=jakub@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).