public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Tamar Christina <tamar.christina@arm.com>
To: binutils@sourceware.org
Cc: nd@arm.com, Richard.Earnshaw@arm.com, marcus.shawcroft@arm.com
Subject: [PATCH][Binutils]AArch64 gas: relax ordering constriants on enabling and disabling feature extensions
Date: Wed, 1 Feb 2023 19:25:23 +0000	[thread overview]
Message-ID: <patch-16852-tamar@arm.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 5734 bytes --]

Hi All,

At the beginning of the port it was decided that enabling features should always
come before disabling features. i.e. +foo should always be before any +nofoo.

For years now this has been relaxed in GCC but binutils has remained rather
strict.  This removes the restriction from gas as well giving users less
friction.

build on native hardware and regtested on
  aarch64-none-elf, aarch64-none-elf (32 bit host),
  aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host)

Cross-compiled and regtested on
  aarch64-none-linux-gnu, aarch64_be-none-linux-gnu

and no issues.

Ok for master?

Thanks,
Tamar

gas/ChangeLog:

2023-02-01  Tamar Christina  <tamar.christina@arm.com>

	* config/tc-aarch64.c (aarch64_parse_features): Remove feature ordering
	checks.
	* testsuite/gas/aarch64/opt_order.l: New test.
	* testsuite/gas/aarch64/opt_order.s: New test.
	* testsuite/gas/aarch64/opt_order_1.d: New test.
	* testsuite/gas/aarch64/opt_order_2.d: New test.
	* testsuite/gas/aarch64/opt_order_3.d: New test.
	* testsuite/gas/aarch64/opt_order_4.d: New test.
	* testsuite/gas/aarch64/opt_order_5.d: New test.
	* testsuite/gas/aarch64/opt_order_6.d: New test.

--- inline copy of patch -- 
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 2aeab6f958ac6435c528b4394412d6395932a6d5..0c2ffd7bb85dbf892f265b6b394de7709de2f60e 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -10234,11 +10234,7 @@ static int
 aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 			bool ext_only)
 {
-  /* We insist on extensions being added before being removed.  We achieve
-     this by using the ADDING_VALUE variable to indicate whether we are
-     adding an extension (1) or removing it (0) and only allowing it to
-     change in the order -1 -> 1 -> 0.  */
-  int adding_value = -1;
+  bool adding_value = false;
   aarch64_feature_set *ext_set = XNEW (aarch64_feature_set);
 
   /* Copy the feature set, so that we can modify it.  */
@@ -10269,31 +10265,20 @@ aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 
       if (optlen >= 2 && startswith (str, "no"))
 	{
-	  if (adding_value != 0)
-	    adding_value = 0;
+	  adding_value = 0;
 	  optlen -= 2;
 	  str += 2;
 	}
       else if (optlen > 0)
 	{
-	  if (adding_value == -1)
-	    adding_value = 1;
-	  else if (adding_value != 1)
-	    {
-	      as_bad (_("must specify extensions to add before specifying "
-			"those to remove"));
-	      return false;
-	    }
+	  adding_value = 1;
 	}
-
-      if (optlen == 0)
+      else if (optlen == 0)
 	{
 	  as_bad (_("missing architectural extension"));
 	  return 0;
 	}
 
-      gas_assert (adding_value != -1);
-
       for (opt = aarch64_features; opt->name != NULL; opt++)
 	if (strncmp (opt->name, str, optlen) == 0)
 	  {
diff --git a/gas/testsuite/gas/aarch64/opt_order.l b/gas/testsuite/gas/aarch64/opt_order.l
new file mode 100644
index 0000000000000000000000000000000000000000..94820513356d2d3c2a02c1f348e2c5423e43f1cd
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.l
@@ -0,0 +1 @@
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order.s b/gas/testsuite/gas/aarch64/opt_order.s
new file mode 100644
index 0000000000000000000000000000000000000000..e0642b3a811ad7d944418f36e237e6fd87771051
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.s
@@ -0,0 +1 @@
+UDOT    V0.2S, V0.8B, V0.8B
diff --git a/gas/testsuite/gas/aarch64/opt_order_1.d b/gas/testsuite/gas/aarch64/opt_order_1.d
new file mode 100644
index 0000000000000000000000000000000000000000..862f7be85f462bfc470a466e9652471d0d268652
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_1.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_2.d b/gas/testsuite/gas/aarch64/opt_order_2.d
new file mode 100644
index 0000000000000000000000000000000000000000..77bd6c57ec9be2e24c3b673f4eccb2c0f7bfc16c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_2.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+nodotprod+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_3.d b/gas/testsuite/gas/aarch64/opt_order_3.d
new file mode 100644
index 0000000000000000000000000000000000000000..5b22090331d11965097a35bad181693c75d51f3c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_3.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_4.d b/gas/testsuite/gas/aarch64/opt_order_4.d
new file mode 100644
index 0000000000000000000000000000000000000000..7ffe2a3011632fdcf25aa32eeb1bedbcc1f408ba
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_4.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_5.d b/gas/testsuite/gas/aarch64/opt_order_5.d
new file mode 100644
index 0000000000000000000000000000000000000000..e7d057c77e7efa14caf55c96cba321e99c36ca21
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_5.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_6.d b/gas/testsuite/gas/aarch64/opt_order_6.d
new file mode 100644
index 0000000000000000000000000000000000000000..b39247de3b3c7261a700b96be8cfbf6b781cbb6b
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_6.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+dotprod+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...




-- 

[-- Attachment #2: rb16852.patch --]
[-- Type: text/plain, Size: 4530 bytes --]

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 2aeab6f958ac6435c528b4394412d6395932a6d5..0c2ffd7bb85dbf892f265b6b394de7709de2f60e 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -10234,11 +10234,7 @@ static int
 aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 			bool ext_only)
 {
-  /* We insist on extensions being added before being removed.  We achieve
-     this by using the ADDING_VALUE variable to indicate whether we are
-     adding an extension (1) or removing it (0) and only allowing it to
-     change in the order -1 -> 1 -> 0.  */
-  int adding_value = -1;
+  bool adding_value = false;
   aarch64_feature_set *ext_set = XNEW (aarch64_feature_set);
 
   /* Copy the feature set, so that we can modify it.  */
@@ -10269,31 +10265,20 @@ aarch64_parse_features (const char *str, const aarch64_feature_set **opt_p,
 
       if (optlen >= 2 && startswith (str, "no"))
 	{
-	  if (adding_value != 0)
-	    adding_value = 0;
+	  adding_value = 0;
 	  optlen -= 2;
 	  str += 2;
 	}
       else if (optlen > 0)
 	{
-	  if (adding_value == -1)
-	    adding_value = 1;
-	  else if (adding_value != 1)
-	    {
-	      as_bad (_("must specify extensions to add before specifying "
-			"those to remove"));
-	      return false;
-	    }
+	  adding_value = 1;
 	}
-
-      if (optlen == 0)
+      else if (optlen == 0)
 	{
 	  as_bad (_("missing architectural extension"));
 	  return 0;
 	}
 
-      gas_assert (adding_value != -1);
-
       for (opt = aarch64_features; opt->name != NULL; opt++)
 	if (strncmp (opt->name, str, optlen) == 0)
 	  {
diff --git a/gas/testsuite/gas/aarch64/opt_order.l b/gas/testsuite/gas/aarch64/opt_order.l
new file mode 100644
index 0000000000000000000000000000000000000000..94820513356d2d3c2a02c1f348e2c5423e43f1cd
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.l
@@ -0,0 +1 @@
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order.s b/gas/testsuite/gas/aarch64/opt_order.s
new file mode 100644
index 0000000000000000000000000000000000000000..e0642b3a811ad7d944418f36e237e6fd87771051
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order.s
@@ -0,0 +1 @@
+UDOT    V0.2S, V0.8B, V0.8B
diff --git a/gas/testsuite/gas/aarch64/opt_order_1.d b/gas/testsuite/gas/aarch64/opt_order_1.d
new file mode 100644
index 0000000000000000000000000000000000000000..862f7be85f462bfc470a466e9652471d0d268652
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_1.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_2.d b/gas/testsuite/gas/aarch64/opt_order_2.d
new file mode 100644
index 0000000000000000000000000000000000000000..77bd6c57ec9be2e24c3b673f4eccb2c0f7bfc16c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_2.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+nodotprod+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_3.d b/gas/testsuite/gas/aarch64/opt_order_3.d
new file mode 100644
index 0000000000000000000000000000000000000000..5b22090331d11965097a35bad181693c75d51f3c
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_3.d
@@ -0,0 +1,4 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+dotprod
+# objdump: -dr
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_4.d b/gas/testsuite/gas/aarch64/opt_order_4.d
new file mode 100644
index 0000000000000000000000000000000000000000..7ffe2a3011632fdcf25aa32eeb1bedbcc1f408ba
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_4.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+nodotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_5.d b/gas/testsuite/gas/aarch64/opt_order_5.d
new file mode 100644
index 0000000000000000000000000000000000000000..e7d057c77e7efa14caf55c96cba321e99c36ca21
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_5.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+dotprod+fp16+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...
diff --git a/gas/testsuite/gas/aarch64/opt_order_6.d b/gas/testsuite/gas/aarch64/opt_order_6.d
new file mode 100644
index 0000000000000000000000000000000000000000..b39247de3b3c7261a700b96be8cfbf6b781cbb6b
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/opt_order_6.d
@@ -0,0 +1,5 @@
+# source: opt_order.s
+# as: -march=armv8.2-a+fp16+dotprod+nodotprod
+# objdump: -dr
+#error_output: opt_order.l
+#...




             reply	other threads:[~2023-02-01 19:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-01 19:25 Tamar Christina [this message]
2023-02-02 11:29 ` Jan Beulich
2023-02-02 12:13   ` Tamar Christina
2023-02-02 12:25     ` Jan Beulich
2023-02-02 13:21   ` [PATCH]AArch64 " Richard Sandiford
2023-02-03  7:13     ` Jan Beulich

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=patch-16852-tamar@arm.com \
    --to=tamar.christina@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=binutils@sourceware.org \
    --cc=marcus.shawcroft@arm.com \
    --cc=nd@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).