public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Victor L. Do Nascimento" <victor.donascimento@arm.com>
To: <gcc-patches@gcc.gnu.org>
Cc: <richard.sandiford@arm.com>, <Kyrylo.Tkachov@arm.com>
Subject: [PATCH] constraint: fix relaxed memory and repeated constraint handling
Date: Thu, 16 Feb 2023 14:27:46 +0000	[thread overview]
Message-ID: <yw8jfsb5fzx9.fsf@arm.com> (raw)

The function `constrain_operands' lacked the logic to consider relaxed
memory constraints when "traditional" memory constraints were not
satisfied, creating potential issues as observed during the reload
compilation pass.

In addition, it was observed that while `constrain_operands' chooses
to disregard constraints when more than one alternative is provided,
e.g. "m,r" using CONSTRAINT__UNKNOWN, it has no checks in place to
determine whether the multiple constraints in a given string are in
fact repetitions of the same constraint and should thus in fact be
treated as a single constraint, as ought to be the case for something
like "m,m".

Both of these issues are dealt with here, thus ensuring that we get
appropriate pattern matching.

Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK to install?

Victor

gcc/
	* lra-constraints.cc (constraint_unique): New.
	(process_address_1): Apply constraint_unique test.
	* recog.cc (constrain_operands): Allow relaxed memory
	constaints.
---
 gcc/lra-constraints.cc | 43 +++++++++++++++++++++++++++++++++++++++---
 gcc/recog.cc           |  3 ++-
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index dbfaf0485..c9c1653c0 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -3448,6 +3448,45 @@ skip_constraint_modifiers (const char *str)
       }
 }
 
+/*  Takes a string of 0 or more comma-separated constraints and the
+    constraint_num correspondig to the first constraint.  When more
+    than one constraint present, evaluate whether they all correspond
+    to a single, repeated constraint (e.g. "r,r") or whether we have
+    more than one distinct constraints (e.g. "r,m").  */
+static bool
+constraint_unique (const char *cstr, enum constraint_num ca)
+{
+   enum constraint_num cb;
+   for (;;)
+     {
+       /* Skip past current constraint and any whitespace which may
+	  precede the end-of-line or separator characters.  */
+       cstr = skip_constraint_modifiers (cstr
+					 + CONSTRAINT_LEN (cstr[0], cstr));
+       /* If end of string reached and no disagreement found, we have
+	  uniqueness.  */
+       if (*cstr == '\0')
+	 return true;
+       /* skip_constraint_modifiers does not handle commas, handle
+	  case manually.  */
+       if (*cstr == ',')
+	 cstr++;
+       /* Get next constraint.  */
+       cstr =  skip_constraint_modifiers (cstr);
+       cb = lookup_constraint ((*cstr == '\0' || *cstr == ',') ? "X" : cstr);
+
+       /* If mismatch found, break out of loop.  */
+       if (cb != ca)
+	 return false;
+
+       /* If *cstr == '\0', we don't want to reach the
+	  skip_constraint_modifiers statement again as that will
+	  advance the pointer past the end of the string.  */
+       if (*cstr == '\0')
+	 return true;
+     }
+}
+
 /* Major function to make reloads for an address in operand NOP or
    check its correctness (If CHECK_ONLY_P is true). The supported
    cases are:
@@ -3507,9 +3546,7 @@ process_address_1 (int nop, bool check_only_p,
      operand has one address constraint, probably all others constraints are
      address ones.  */
   if (constraint[0] != '\0' && get_constraint_type (cn) != CT_ADDRESS
-      && *skip_constraint_modifiers (constraint
-				     + CONSTRAINT_LEN (constraint[0],
-						       constraint)) != '\0')
+      && !constraint_unique (constraint, cn))
     cn = CONSTRAINT__UNKNOWN;
   if (insn_extra_address_constraint (cn)
       /* When we find an asm operand with an address constraint that
diff --git a/gcc/recog.cc b/gcc/recog.cc
index 200cf4214..3ddeab59d 100644
--- a/gcc/recog.cc
+++ b/gcc/recog.cc
@@ -3234,7 +3234,8 @@ constrain_operands (int strict, alternative_mask alternatives)
 		  else if (constraint_satisfied_p (op, cn))
 		    win = 1;
 
-		  else if (insn_extra_memory_constraint (cn)
+		  else if ((insn_extra_memory_constraint (cn)
+			    || insn_extra_relaxed_memory_constraint (cn))
 			   /* Every memory operand can be reloaded to fit.  */
 			   && ((strict < 0 && MEM_P (op))
 			       /* Before reload, accept what reload can turn
-- 
2.36.1



             reply	other threads:[~2023-02-16 14:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-16 14:27 Victor L. Do Nascimento [this message]
2023-02-27 14:29 ` Richard Sandiford
2023-04-18 10:13 Victor L. Do Nascimento
2023-04-18 11:14 ` Richard Sandiford

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=yw8jfsb5fzx9.fsf@arm.com \
    --to=victor.donascimento@arm.com \
    --cc=Kyrylo.Tkachov@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.sandiford@arm.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).