public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Roger Sayle <sayle@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r12-7046] [PATCH] PR rtl-optimization/101885: Prevent combine from clobbering flags
Date: Fri,  4 Feb 2022 09:38:30 +0000 (GMT)	[thread overview]
Message-ID: <20220204093830.91C7D3858D20@sourceware.org> (raw)

https://gcc.gnu.org/g:49365d511ac9b64009b1de11ef8a941f59407f67

commit r12-7046-g49365d511ac9b64009b1de11ef8a941f59407f67
Author: Roger Sayle <roger@nextmovesoftware.com>
Date:   Fri Feb 4 09:32:21 2022 +0000

    [PATCH] PR rtl-optimization/101885: Prevent combine from clobbering flags
    
    This patch addresses PR rtl-optimization/101885 which is a P2 wrong code
    regression.  In combine, if the resulting fused instruction is a parallel
    of two sets which fails to be recognized by the backend, combine tries to
    emit these as two sequential set instructions (known as split_i2i3).
    As each set is recognized the backend may add any necessary "clobbers".
    The code currently checks that any clobbers added to the first "set"
    don't interfere with the second set, but doesn't currently handle the
    case that clobbers added to the second set may interfere/kill the
    destination of the first set (which must be live at this point).
    The solution is to cut'n'paste the "clobber" logic from just a few
    lines earlier, suitably adjusted for the second instruction.
    
    One minor nit that may confuse a reviewer is that at this point in
    the code we've lost track of which set was first and which was second
    (combine chooses dynamically, and the recog processes that adds the
    clobbers may have obfuscated the original SET_DEST) so the actual test
    below is to confirm that any newly added clobbers (on the second set
    instruction) don't overlap either set0's or set1's destination.
    
    2022-02-04  Roger Sayle  <roger@nextmovesoftware.com>
    
    gcc/ChangeLog
            PR rtl-optimization/101885
            * combine.cc (try_combine): When splitting a parallel into two
            sequential sets, check not only that the first doesn't clobber
            the second but also that the second doesn't clobber the first.
    
    gcc/testsuite/ChangeLog
            PR rtl-optimization/101885
            * gcc.dg/pr101885.c: New test case.

Diff:
---
 gcc/combine.cc                  | 18 ++++++++++++++++++
 gcc/testsuite/gcc.dg/pr101885.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 2d406354735..7683f8250fe 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -4017,6 +4017,24 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 
 	  insn_code_number = recog_for_combine (&newpat, i3, &new_i3_notes);
 
+	  /* Likewise, recog_for_combine might have added clobbers to NEWPAT.
+	     Checking that the SET0's SET_DEST and SET1's SET_DEST aren't
+	     mentioned/clobbered, ensures NEWI2PAT's SET_DEST is live.  */
+	  if (insn_code_number >= 0 && GET_CODE (newpat) == PARALLEL)
+	    {
+	      for (i = XVECLEN (newpat, 0) - 1; i >= 0; i--)
+		if (GET_CODE (XVECEXP (newpat, 0, i)) == CLOBBER)
+		  {
+		    rtx reg = XEXP (XVECEXP (newpat, 0, i), 0);
+		    if (reg_overlap_mentioned_p (reg, SET_DEST (set0))
+			|| reg_overlap_mentioned_p (reg, SET_DEST (set1)))
+		      {
+			undo_all ();
+			return 0;
+		      }
+		  }
+	    }
+
 	  if (insn_code_number >= 0)
 	    split_i2i3 = 1;
 	}
diff --git a/gcc/testsuite/gcc.dg/pr101885.c b/gcc/testsuite/gcc.dg/pr101885.c
new file mode 100644
index 00000000000..05fd0ed3080
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr101885.c
@@ -0,0 +1,31 @@
+/* { dg-do run } */
+/* { dg-options "-O3" } */
+int a = 3, c;
+short b = 5, d, f;
+volatile short e;
+
+__attribute__((noipa)) void
+foo (void)
+{
+}
+
+int
+main ()
+{
+  for (f = 0; f != 2; f++)
+    {
+      int g = a;
+      if (b)
+	if (a)
+	  for (a = b = 0; b <= 3; b++)
+	    ;
+      for (c = 0; c != 16; ++c)
+	e;
+    }
+  b = d;
+  foo ();
+  if (a != 0)
+    __builtin_abort ();
+  return 0;
+}
+


                 reply	other threads:[~2022-02-04  9: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=20220204093830.91C7D3858D20@sourceware.org \
    --to=sayle@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: link
Be 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).