public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][AArch64][2/2] Add sve_width -moverride tunable
@ 2018-12-07 15:44 Kyrill Tkachov
  2018-12-07 16:32 ` Richard Sandiford
  0 siblings, 1 reply; 3+ messages in thread
From: Kyrill Tkachov @ 2018-12-07 15:44 UTC (permalink / raw)
  To: gcc-patches
  Cc: Marcus Shawcroft, Richard Earnshaw (lists),
	James Greenhalgh, Richard Sandiford

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

Hi all,

On top of the previous patch that implements TARGET_ESTIMATED_POLY_VALUE
and adds an sve_width tuning field to the CPU structs, this patch implements
an -moverride knob to adjust this sve_width field to allow for experimentation.
Again, reminder that this only has an effect when compiling for VLA-SVE that is,
without msve-vector-bits=<foo>. This just adjusts tuning heuristics in the compiler,,
like profitability thresholds for vectorised versioned loops, and others.

It can be used, for example like -moverride=sve_width=256 to set the sve_width
tuning field to 256. Widths outside of the accepted SVE widths [128 - 2048] are rejected
as you'd expect.

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk?

Thanks,
Kyrill

2018-12-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.c (aarch64_tuning_override_functions): Add
     sve_width entry.
     (aarch64_parse_sve_width_string): Define.

2018-12-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/aarch64/sve/override_sve_width_1.c: New test.

[-- Attachment #2: sve-width-2.patch --]
[-- Type: text/x-patch, Size: 2370 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 7ccc6b78d5872d6b43491badbfa9f2d70580015c..bad687d33479f4b6f8cbeaca799824e29b8e9ed1 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1086,12 +1086,14 @@ struct aarch64_tuning_override_function
 
 static void aarch64_parse_fuse_string (const char*, struct tune_params*);
 static void aarch64_parse_tune_string (const char*, struct tune_params*);
+static void aarch64_parse_sve_width_string (const char*, struct tune_params*);
 
 static const struct aarch64_tuning_override_function
 aarch64_tuning_override_functions[] =
 {
   { "fuse", aarch64_parse_fuse_string },
   { "tune", aarch64_parse_tune_string },
+  { "sve_width", aarch64_parse_sve_width_string },
   { NULL, NULL }
 };
 
@@ -10834,6 +10836,34 @@ aarch64_parse_tune_string (const char *tune_string,
 				     "tune=");
 }
 
+/* Parse the sve_width tuning moverride string in TUNE_STRING.
+   Accept the valid SVE vector widths allowed by
+   aarch64_sve_vector_bits_enum and use it to override sve_width
+   in TUNE.  */
+
+static void
+aarch64_parse_sve_width_string (const char *tune_string,
+				struct tune_params *tune)
+{
+  int width = -1;
+
+  int n = sscanf (tune_string, "%d", &width);
+  if (n == EOF)
+    error ("invalid format for sve_width");
+  switch (width)
+    {
+      case SVE_128:
+      case SVE_256:
+      case SVE_512:
+      case SVE_1024:
+      case SVE_2048:
+	break;
+      default:
+	error ("invalid sve_width value: %d", width);
+    }
+  tune->sve_width = (enum aarch64_sve_vector_bits_enum) width;
+}
+
 /* Parse TOKEN, which has length LENGTH to see if it is a tuning option
    we understand.  If it is, extract the option string and handoff to
    the appropriate function.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/override_sve_width_1.c b/gcc/testsuite/gcc.target/aarch64/sve/override_sve_width_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3752fdc2a7198783d2ed5c5f502c3227f98029b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/override_sve_width_1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -moverride=sve_width=512" } */
+
+void __attribute__((noinline, noclone))
+vadd (int *dst, int *op1, int *op2, int count)
+{
+  for (int i = 0; i < count; ++i)
+    dst[i] = op1[i] + op2[i];
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][AArch64][2/2] Add sve_width -moverride tunable
  2018-12-07 15:44 [PATCH][AArch64][2/2] Add sve_width -moverride tunable Kyrill Tkachov
@ 2018-12-07 16:32 ` Richard Sandiford
  2018-12-07 17:06   ` Kyrill Tkachov
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Sandiford @ 2018-12-07 16:32 UTC (permalink / raw)
  To: Kyrill Tkachov
  Cc: gcc-patches, Marcus Shawcroft, Richard Earnshaw (lists),
	James Greenhalgh

"Kyrill Tkachov" <kyrylo.tkachov@foss.arm.com> writes:
> @@ -10834,6 +10836,34 @@ aarch64_parse_tune_string (const char *tune_string,
>  				     "tune=");
>  }
>  
> +/* Parse the sve_width tuning moverride string in TUNE_STRING.
> +   Accept the valid SVE vector widths allowed by
> +   aarch64_sve_vector_bits_enum and use it to override sve_width
> +   in TUNE.  */
> +
> +static void
> +aarch64_parse_sve_width_string (const char *tune_string,
> +				struct tune_params *tune)
> +{
> +  int width = -1;
> +
> +  int n = sscanf (tune_string, "%d", &width);
> +  if (n == EOF)
> +    error ("invalid format for sve_width");

Should probably return here, otherwise we'll report a second
error for width == -1.

> +  switch (width)
> +    {
> +      case SVE_128:
> +      case SVE_256:
> +      case SVE_512:
> +      case SVE_1024:
> +      case SVE_2048:
> +	break;
> +      default:

> +	error ("invalid sve_width value: %d", width);
> +    }
> +  tune->sve_width = (enum aarch64_sve_vector_bits_enum) width;
> +}

Formatting nit: cases should line up with the "{".

OK with those changes, thanks.

Richard

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][AArch64][2/2] Add sve_width -moverride tunable
  2018-12-07 16:32 ` Richard Sandiford
@ 2018-12-07 17:06   ` Kyrill Tkachov
  0 siblings, 0 replies; 3+ messages in thread
From: Kyrill Tkachov @ 2018-12-07 17:06 UTC (permalink / raw)
  To: gcc-patches, Marcus Shawcroft, Richard Earnshaw (lists),
	James Greenhalgh, richard.sandiford

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


On 07/12/18 16:32, Richard Sandiford wrote:
> "Kyrill Tkachov" <kyrylo.tkachov@foss.arm.com> writes:
>> @@ -10834,6 +10836,34 @@ aarch64_parse_tune_string (const char *tune_string,
>>   				     "tune=");
>>   }
>>   
>> +/* Parse the sve_width tuning moverride string in TUNE_STRING.
>> +   Accept the valid SVE vector widths allowed by
>> +   aarch64_sve_vector_bits_enum and use it to override sve_width
>> +   in TUNE.  */
>> +
>> +static void
>> +aarch64_parse_sve_width_string (const char *tune_string,
>> +				struct tune_params *tune)
>> +{
>> +  int width = -1;
>> +
>> +  int n = sscanf (tune_string, "%d", &width);
>> +  if (n == EOF)
>> +    error ("invalid format for sve_width");
> Should probably return here, otherwise we'll report a second
> error for width == -1.
>
>> +  switch (width)
>> +    {
>> +      case SVE_128:
>> +      case SVE_256:
>> +      case SVE_512:
>> +      case SVE_1024:
>> +      case SVE_2048:
>> +	break;
>> +      default:
>> +	error ("invalid sve_width value: %d", width);
>> +    }
>> +  tune->sve_width = (enum aarch64_sve_vector_bits_enum) width;
>> +}
> Formatting nit: cases should line up with the "{".
>
> OK with those changes, thanks.

Thanks Richard. This is what I've committed with r266898.

Kyrill

> Richard


[-- Attachment #2: sve-width-3.patch --]
[-- Type: text/x-patch, Size: 2399 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 7ccc6b78d5872d6b43491badbfa9f2d70580015c..da050b88bdc4859e6c3eb7f90023a05868536399 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1086,12 +1086,14 @@ struct aarch64_tuning_override_function
 
 static void aarch64_parse_fuse_string (const char*, struct tune_params*);
 static void aarch64_parse_tune_string (const char*, struct tune_params*);
+static void aarch64_parse_sve_width_string (const char*, struct tune_params*);
 
 static const struct aarch64_tuning_override_function
 aarch64_tuning_override_functions[] =
 {
   { "fuse", aarch64_parse_fuse_string },
   { "tune", aarch64_parse_tune_string },
+  { "sve_width", aarch64_parse_sve_width_string },
   { NULL, NULL }
 };
 
@@ -10834,6 +10836,37 @@ aarch64_parse_tune_string (const char *tune_string,
 				     "tune=");
 }
 
+/* Parse the sve_width tuning moverride string in TUNE_STRING.
+   Accept the valid SVE vector widths allowed by
+   aarch64_sve_vector_bits_enum and use it to override sve_width
+   in TUNE.  */
+
+static void
+aarch64_parse_sve_width_string (const char *tune_string,
+				struct tune_params *tune)
+{
+  int width = -1;
+
+  int n = sscanf (tune_string, "%d", &width);
+  if (n == EOF)
+    {
+      error ("invalid format for sve_width");
+      return;
+    }
+  switch (width)
+    {
+    case SVE_128:
+    case SVE_256:
+    case SVE_512:
+    case SVE_1024:
+    case SVE_2048:
+      break;
+    default:
+      error ("invalid sve_width value: %d", width);
+    }
+  tune->sve_width = (enum aarch64_sve_vector_bits_enum) width;
+}
+
 /* Parse TOKEN, which has length LENGTH to see if it is a tuning option
    we understand.  If it is, extract the option string and handoff to
    the appropriate function.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/override_sve_width_1.c b/gcc/testsuite/gcc.target/aarch64/sve/override_sve_width_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3752fdc2a7198783d2ed5c5f502c3227f98029b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/override_sve_width_1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -moverride=sve_width=512" } */
+
+void __attribute__((noinline, noclone))
+vadd (int *dst, int *op1, int *op2, int count)
+{
+  for (int i = 0; i < count; ++i)
+    dst[i] = op1[i] + op2[i];
+}

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-12-07 17:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-07 15:44 [PATCH][AArch64][2/2] Add sve_width -moverride tunable Kyrill Tkachov
2018-12-07 16:32 ` Richard Sandiford
2018-12-07 17:06   ` Kyrill Tkachov

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).