public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
From: "ubizjak at gmail dot com" <gcc-bugzilla@gcc.gnu.org> To: gcc-bugs@gcc.gnu.org Subject: [Bug target/100182] [8/9/10/11/12 Regression] Miscompilation of atomic_float/1.cc and atomic_float/wait_notify.cc on i686 Date: Thu, 22 Apr 2021 17:53:26 +0000 [thread overview] Message-ID: <bug-100182-4-afi9V9RVNc@http.gcc.gnu.org/bugzilla/> (raw) In-Reply-To: <bug-100182-4@http.gcc.gnu.org/bugzilla/> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100182 Uroš Bizjak <ubizjak at gmail dot com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC|uros at gcc dot gnu.org | Assignee|unassigned at gcc dot gnu.org |ubizjak at gmail dot com --- Comment #7 from Uroš Bizjak <ubizjak at gmail dot com> --- (In reply to Jakub Jelinek from comment #1) > In this particular case it is the sync.md:398 peephole2: > (define_peephole2 > [(set (match_operand:DF 0 "memory_operand") > (match_operand:DF 1 "any_fp_register_operand")) > (set (mem:BLK (scratch:SI)) > (unspec:BLK [(mem:BLK (scratch:SI))] UNSPEC_MEMORY_BLOCKAGE)) > (set (match_operand:DF 2 "fp_register_operand") > (unspec:DF [(match_operand:DI 3 "memory_operand")] > UNSPEC_FILD_ATOMIC)) > (set (match_operand:DI 4 "memory_operand") > (unspec:DI [(match_dup 2)] > UNSPEC_FIST_ATOMIC))] > "!TARGET_64BIT > && peep2_reg_dead_p (4, operands[2]) > && rtx_equal_p (XEXP (operands[0], 0), XEXP (operands[3], 0))" > [(const_int 0)] > { > emit_insn (gen_memory_blockage ()); > emit_move_insn (gen_lowpart (DFmode, operands[4]), operands[1]); > DONE; > }) > that triggers here but from what I can read, all the r7-1112 peephole2s > optimize away stores to some memory on the assumption that the memory is > read only once (in another insn matched by the same peephole2). > I'm not 100% sure if we can rely for it on spill slots for which r7-112 > seems to have been written, but for other memory we'd need to prove that the > memory is dead. > Rather than removing those peephole2s altogether, I wonder if we just > shouldn't check that the memory_operand which we'd optimize away stores to > aren't spill slots. Actually, these peepholes are too eager and also remove the store to the memory operand 0 on the assumption that the operand is used only in the peephole2 sequence. As shown in the testcase, this is not always true, and operand 0 can be accessed also after the peephole2'd sequence. The solution is to not remove the store to operand 0. Probably there will be some unneeded stores left in the code, but IMO, this is a small price to pay for the correctness. And we still remove fild/fistp pair. I'm testing the following patch: --cut here-- diff --git a/gcc/config/i386/sync.md b/gcc/config/i386/sync.md index c7c508c8de8..538d1f89497 100644 --- a/gcc/config/i386/sync.md +++ b/gcc/config/i386/sync.md @@ -392,7 +392,8 @@ "!TARGET_64BIT && peep2_reg_dead_p (3, operands[2]) && rtx_equal_p (XEXP (operands[0], 0), XEXP (operands[3], 0))" - [(set (match_dup 5) (match_dup 1))] + [(set (match_dup 0) (match_dup 1)) + (set (match_dup 5) (match_dup 1))] "operands[5] = gen_lowpart (DFmode, operands[4]);") (define_peephole2 @@ -411,6 +412,7 @@ && rtx_equal_p (XEXP (operands[0], 0), XEXP (operands[3], 0))" [(const_int 0)] { + emit_move_insn (operands[0], operands[1]); emit_insn (gen_memory_blockage ()); emit_move_insn (gen_lowpart (DFmode, operands[4]), operands[1]); DONE; @@ -428,7 +430,8 @@ "!TARGET_64BIT && peep2_reg_dead_p (3, operands[2]) && rtx_equal_p (XEXP (operands[0], 0), XEXP (operands[3], 0))" - [(set (match_dup 5) (match_dup 1))] + [(set (match_dup 0) (match_dup 1)) + (set (match_dup 5) (match_dup 1))] "operands[5] = gen_lowpart (DFmode, operands[4]);") (define_peephole2 @@ -447,6 +450,7 @@ && rtx_equal_p (XEXP (operands[0], 0), XEXP (operands[3], 0))" [(const_int 0)] { + emit_move_insn (operands[0], operands[1]); emit_insn (gen_memory_blockage ()); emit_move_insn (gen_lowpart (DFmode, operands[4]), operands[1]); DONE; --cut here--
next prev parent reply other threads:[~2021-04-22 17:53 UTC|newest] Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-21 12:35 [Bug target/100182] New: [8/9/10/11/12 Regression] Miscompilation of atomic_float/1.cc " jakub at gcc dot gnu.org 2021-04-21 12:35 ` [Bug target/100182] " jakub at gcc dot gnu.org 2021-04-21 14:23 ` jakub at gcc dot gnu.org 2021-04-21 15:35 ` jakub at gcc dot gnu.org 2021-04-21 15:41 ` jakub at gcc dot gnu.org 2021-04-21 15:42 ` jakub at gcc dot gnu.org 2021-04-22 8:28 ` jakub at gcc dot gnu.org 2021-04-22 13:10 ` [Bug target/100182] [8/9/10/11/12 Regression] Miscompilation of atomic_float/1.cc and atomic_float/wait_notify.cc " cvs-commit at gcc dot gnu.org 2021-04-22 13:10 ` cvs-commit at gcc dot gnu.org 2021-04-22 17:53 ` ubizjak at gmail dot com [this message] 2021-04-22 17:58 ` jakub at gcc dot gnu.org 2021-04-22 18:09 ` ubizjak at gmail dot com 2021-04-22 18:41 ` ubizjak at gmail dot com 2021-04-23 6:13 ` ubizjak at gmail dot com 2021-04-23 7:40 ` jakub at gcc dot gnu.org 2021-04-23 7:52 ` ubizjak at gmail dot com 2021-04-23 7:54 ` ubizjak at gmail dot com 2021-04-23 7:56 ` jakub at gcc dot gnu.org 2021-04-23 8:02 ` ubizjak at gmail dot com 2021-04-23 8:13 ` ubizjak at gmail dot com 2021-04-23 8:25 ` jakub at gcc dot gnu.org 2021-04-23 8:36 ` jakub at gcc dot gnu.org 2021-04-23 8:41 ` jakub at gcc dot gnu.org 2021-04-23 9:20 ` ubizjak at gmail dot com 2021-04-23 15:30 ` cvs-commit at gcc dot gnu.org 2021-04-28 10:44 ` cvs-commit at gcc dot gnu.org 2021-04-28 13:33 ` cvs-commit at gcc dot gnu.org 2021-04-28 18:02 ` cvs-commit at gcc dot gnu.org 2021-04-28 18:02 ` cvs-commit at gcc dot gnu.org 2021-04-28 18:09 ` ubizjak at gmail dot com 2021-07-19 13:08 ` hjl.tools at gmail dot com 2021-07-19 14:40 ` ubizjak at gmail dot com 2021-07-19 22:06 ` hjl.tools at gmail dot com 2021-07-19 22:18 ` ubizjak at gmail dot com 2021-07-20 4:23 ` cvs-commit at gcc dot gnu.org 2021-07-20 4:30 ` cvs-commit at gcc dot gnu.org 2021-07-20 4:36 ` cvs-commit at gcc dot gnu.org 2021-07-20 4:39 ` cvs-commit at gcc dot gnu.org 2021-07-20 4:41 ` ubizjak at gmail dot com 2021-07-31 19:24 ` hjl.tools at gmail dot com 2021-08-03 18:14 ` hjl.tools at gmail dot com 2021-08-03 18:24 ` ubizjak at gmail dot com 2021-08-03 18:30 ` hjl.tools at gmail dot com
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=bug-100182-4-afi9V9RVNc@http.gcc.gnu.org/bugzilla/ \ --to=gcc-bugzilla@gcc.gnu.org \ --cc=gcc-bugs@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).