public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Manolis Tsamis <manolis.tsamis@vrull.eu>
To: gcc-patches@gcc.gnu.org
Cc: Jakub Jelinek <jakub@redhat.com>,
	Philipp Tomsich <philipp.tomsich@vrull.eu>,
	Robin Dapp <rdapp@linux.ibm.com>,
	Andrew Pinski <apinski@marvell.com>,
	Manolis Tsamis <manolis.tsamis@vrull.eu>
Subject: [PATCH 1/2] ifcvt: handle sequences that clobber flags in noce_convert_multiple_sets
Date: Sat,  1 Jul 2023 11:24:12 +0200	[thread overview]
Message-ID: <20230701092413.2488806-2-manolis.tsamis@vrull.eu> (raw)
In-Reply-To: <20230701092413.2488806-1-manolis.tsamis@vrull.eu>

This is an extension of what was done in PR106590.

Currently if a sequence generated in noce_convert_multiple_sets clobbers the
condition rtx (cc_cmp or rev_cc_cmp) then only seq1 is used afterwards
(sequences that emit the comparison itself). Since this applies only from the
next iteration it assumes that the sequences generated (in particular seq2)
doesn't clobber the condition rtx itself before using it in the if_then_else,
which is only true in specific cases (currently only register/subregister moves
are allowed).

This patch changes this so it also tests if seq2 clobbers cc_cmp/rev_cc_cmp in
the current iteration. This makes it possible to include arithmetic operations
in noce_convert_multiple_sets.

gcc/ChangeLog:

	* ifcvt.cc (check_for_cc_cmp_clobbers): Use modified_in_p instead.
	(noce_convert_multiple_sets_1): Don't use seq2 if it clobbers cc_cmp.

Signed-off-by: Manolis Tsamis <manolis.tsamis@vrull.eu>
---

 gcc/ifcvt.cc | 49 +++++++++++++++++++------------------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index 0b180b4568f..fd1ce8a1049 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -3373,20 +3373,6 @@ noce_convert_multiple_sets (struct noce_if_info *if_info)
   return TRUE;
 }
 
-/* Helper function for noce_convert_multiple_sets_1.  If store to
-   DEST can affect P[0] or P[1], clear P[0].  Called via note_stores.  */
-
-static void
-check_for_cc_cmp_clobbers (rtx dest, const_rtx, void *p0)
-{
-  rtx *p = (rtx *) p0;
-  if (p[0] == NULL_RTX)
-    return;
-  if (reg_overlap_mentioned_p (dest, p[0])
-      || (p[1] && reg_overlap_mentioned_p (dest, p[1])))
-    p[0] = NULL_RTX;
-}
-
 /* This goes through all relevant insns of IF_INFO->then_bb and tries to
    create conditional moves.  In case a simple move sufficis the insn
    should be listed in NEED_NO_CMOV.  The rewired-src cases should be
@@ -3550,9 +3536,17 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info,
 	 creating an additional compare for each.  If successful, costing
 	 is easier and this sequence is usually preferred.  */
       if (cc_cmp)
-	seq2 = try_emit_cmove_seq (if_info, temp, cond,
-				   new_val, old_val, need_cmov,
-				   &cost2, &temp_dest2, cc_cmp, rev_cc_cmp);
+	{
+	  seq2 = try_emit_cmove_seq (if_info, temp, cond,
+				     new_val, old_val, need_cmov,
+				     &cost2, &temp_dest2, cc_cmp, rev_cc_cmp);
+
+	  /* The if_then_else in SEQ2 may be affected when cc_cmp/rev_cc_cmp is
+	     clobbered.  We can't safely use the sequence in this case.  */
+	  if (seq2 && (modified_in_p (cc_cmp, seq2)
+	      || (rev_cc_cmp && modified_in_p (rev_cc_cmp, seq2))))
+	    seq2 = NULL;
+	}
 
       /* The backend might have created a sequence that uses the
 	 condition.  Check this.  */
@@ -3607,21 +3601,16 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info,
 	  return FALSE;
 	}
 
-      if (cc_cmp)
+      if (cc_cmp && seq == seq1)
 	{
-	  /* Check if SEQ can clobber registers mentioned in
-	     cc_cmp and/or rev_cc_cmp.  If yes, we need to use
-	     only seq1 from that point on.  */
-	  rtx cc_cmp_pair[2] = { cc_cmp, rev_cc_cmp };
-	  for (walk = seq; walk; walk = NEXT_INSN (walk))
+	  /* Check if SEQ can clobber registers mentioned in cc_cmp/rev_cc_cmp.
+	     If yes, we need to use only seq1 from that point on.
+	     Only check when we use seq1 since we have already tested seq2.  */
+	  if (modified_in_p (cc_cmp, seq)
+	      || (rev_cc_cmp && modified_in_p (rev_cc_cmp, seq)))
 	    {
-	      note_stores (walk, check_for_cc_cmp_clobbers, cc_cmp_pair);
-	      if (cc_cmp_pair[0] == NULL_RTX)
-		{
-		  cc_cmp = NULL_RTX;
-		  rev_cc_cmp = NULL_RTX;
-		  break;
-		}
+	      cc_cmp = NULL_RTX;
+	      rev_cc_cmp = NULL_RTX;
 	    }
 	}
 
-- 
2.34.1


  reply	other threads:[~2023-07-01  9:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-01  9:24 [PATCH 0/2] ifcvt: Allow if conversion of arithmetic in basic blocks with multiple sets Manolis Tsamis
2023-07-01  9:24 ` Manolis Tsamis [this message]
2023-07-01  9:24 ` [PATCH 2/2] ifcvt: Allow more operations in multiple set if conversion Manolis Tsamis
2023-07-03  9:12   ` Robin Dapp
2023-07-04 14:32     ` Manolis Tsamis
2023-07-13 14:11       ` Manolis Tsamis

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=20230701092413.2488806-2-manolis.tsamis@vrull.eu \
    --to=manolis.tsamis@vrull.eu \
    --cc=apinski@marvell.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=philipp.tomsich@vrull.eu \
    --cc=rdapp@linux.ibm.com \
    /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).