[PATCH] Fix up ARM target attribute handling (PR target/89093)

[PATCH] Fix up ARM target attribute handling (PR target/89093)

[Jakub Jelinek]

Hi!

Just something I've noticed while looking at Ramana's patch.

As can be seen on the testcase, on arm we accept arbitrary garbage
after arm or thumb prefixes, is that really desirable?
While for fpu= or arch= we reject garbage after it and so do for
target attribute arg starting with +.

Ok if this passes bootstrap/regtest?

Note, I don't understand the while (ISSPACE (*q)) ++q; there (aarch64
does the same), do we really want to support
__attribute__((target ("   arm"))) ?
Looked at other targets and can't see anything like that being supported
elsewhere.

2019-04-12  Jakub Jelinek  <jakub@redhat.com>

	PR target/89093
	* config/arm/arm.c (arm_valid_target_attribute_rec): Use strcmp
	instead of strncmp when checking for thumb and arm.  Formatting fixes.

	* gcc.target/arm/pr89093.c: New test.

--- gcc/config/arm/arm.c.jj	2019-04-09 15:18:37.879816537 +0200
+++ gcc/config/arm/arm.c	2019-04-12 15:36:36.993102230 +0200
@@ -30874,16 +30874,16 @@ arm_valid_target_attribute_rec (tree arg
       while (ISSPACE (*q)) ++q;
 
       argstr = NULL;
-      if (!strncmp (q, "thumb", 5))
-	  opts->x_target_flags |= MASK_THUMB;
+      if (!strcmp (q, "thumb"))
+	opts->x_target_flags |= MASK_THUMB;
 
-      else if (!strncmp (q, "arm", 3))
-	  opts->x_target_flags &= ~MASK_THUMB;
+      else if (!strcmp (q, "arm"))
+	opts->x_target_flags &= ~MASK_THUMB;
 
       else if (!strncmp (q, "fpu=", 4))
 	{
 	  int fpu_index;
-	  if (! opt_enum_arg_to_value (OPT_mfpu_, q+4,
+	  if (! opt_enum_arg_to_value (OPT_mfpu_, q + 4,
 				       &fpu_index, CL_TARGET))
 	    {
 	      error ("invalid fpu for target attribute or pragma %qs", q);
@@ -30901,7 +30901,7 @@ arm_valid_target_attribute_rec (tree arg
 	}
       else if (!strncmp (q, "arch=", 5))
 	{
-	  char* arch = q+5;
+	  char *arch = q + 5;
 	  const arch_option *arm_selected_arch
 	     = arm_parse_arch_option_name (all_architectures, "arch", arch);
 
--- gcc/testsuite/gcc.target/arm/pr89093.c.jj	2019-04-12 16:05:47.477069147 +0200
+++ gcc/testsuite/gcc.target/arm/pr89093.c	2019-04-12 16:05:15.948591951 +0200
@@ -0,0 +1,7 @@
+/* PR target/89093 */
+/* { dg-do compile } */
+
+__attribute__((target ("arm.foobar"))) void f1 (void) {} /* { dg-error "unknown target attribute or pragma 'arm.foobar'" } */
+__attribute__((target ("thumbozoo1"))) void f2 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbozoo1'" } */
+__attribute__((target ("arm,thumbique"))) void f3 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbique'" } */
+__attribute__((target ("thumb981,arm"))) void f4 (void) {} /* { dg-error "unknown target attribute or pragma 'thumb981'" } */

	Jakub

Re: [PATCH] Fix up ARM target attribute handling (PR target/89093)

[Ramana Radhakrishnan]

On 12/04/2019 15:12, Jakub Jelinek wrote:
> Hi!
> 
> Just something I've noticed while looking at Ramana's patch.
> 
> As can be seen on the testcase, on arm we accept arbitrary garbage
> after arm or thumb prefixes, is that really desirable?
> While for fpu= or arch= we reject garbage after it and so do for
> target attribute arg starting with +.
> 
> Ok if this passes bootstrap/regtest?
> 

Bah, that's not supposed to be there, Yes that's ok .

> Note, I don't understand the while (ISSPACE (*q)) ++q; there (aarch64
> does the same), do we really want to support
> __attribute__((target ("   arm"))) ?
> Looked at other targets and can't see anything like that being supported
> elsewhere.

No, that's not right. we should get rid of this.

Thanks a lot for fixing this up Jakub.

Ramana

> 
> 2019-04-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/89093
> 	* config/arm/arm.c (arm_valid_target_attribute_rec): Use strcmp
> 	instead of strncmp when checking for thumb and arm.  Formatting fixes.
> 
> 	* gcc.target/arm/pr89093.c: New test.
> 
> --- gcc/config/arm/arm.c.jj	2019-04-09 15:18:37.879816537 +0200
> +++ gcc/config/arm/arm.c	2019-04-12 15:36:36.993102230 +0200
> @@ -30874,16 +30874,16 @@ arm_valid_target_attribute_rec (tree arg
>         while (ISSPACE (*q)) ++q;
>   
>         argstr = NULL;
> -      if (!strncmp (q, "thumb", 5))
> -	  opts->x_target_flags |= MASK_THUMB;
> +      if (!strcmp (q, "thumb"))
> +	opts->x_target_flags |= MASK_THUMB;
>   
> -      else if (!strncmp (q, "arm", 3))
> -	  opts->x_target_flags &= ~MASK_THUMB;
> +      else if (!strcmp (q, "arm"))
> +	opts->x_target_flags &= ~MASK_THUMB;
>   
>         else if (!strncmp (q, "fpu=", 4))
>   	{
>   	  int fpu_index;
> -	  if (! opt_enum_arg_to_value (OPT_mfpu_, q+4,
> +	  if (! opt_enum_arg_to_value (OPT_mfpu_, q + 4,
>   				       &fpu_index, CL_TARGET))
>   	    {
>   	      error ("invalid fpu for target attribute or pragma %qs", q);
> @@ -30901,7 +30901,7 @@ arm_valid_target_attribute_rec (tree arg
>   	}
>         else if (!strncmp (q, "arch=", 5))
>   	{
> -	  char* arch = q+5;
> +	  char *arch = q + 5;
>   	  const arch_option *arm_selected_arch
>   	     = arm_parse_arch_option_name (all_architectures, "arch", arch);
>   
> --- gcc/testsuite/gcc.target/arm/pr89093.c.jj	2019-04-12 16:05:47.477069147 +0200
> +++ gcc/testsuite/gcc.target/arm/pr89093.c	2019-04-12 16:05:15.948591951 +0200
> @@ -0,0 +1,7 @@
> +/* PR target/89093 */
> +/* { dg-do compile } */
> +
> +__attribute__((target ("arm.foobar"))) void f1 (void) {} /* { dg-error "unknown target attribute or pragma 'arm.foobar'" } */
> +__attribute__((target ("thumbozoo1"))) void f2 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbozoo1'" } */
> +__attribute__((target ("arm,thumbique"))) void f3 (void) {} /* { dg-error "unknown target attribute or pragma 'thumbique'" } */
> +__attribute__((target ("thumb981,arm"))) void f4 (void) {} /* { dg-error "unknown target attribute or pragma 'thumb981'" } */
> 
> 	Jakub
> 

[PATCH] Don't ignore leading whitespace in ARM target attribute/pragma (PR target/89093)

[Jakub Jelinek]

On Fri, Apr 12, 2019 at 05:10:48PM +0100, Ramana Radhakrishnan wrote:
> No, that's not right. we should get rid of this.

Here is a patch for that.

Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?

2019-04-16  Jakub Jelinek  <jakub@redhat.com>

	PR target/89093
	* config/arm/arm.c (arm_valid_target_attribute_rec): Don't skip
	whitespace at the start of target attribute string.

	* gcc.target/arm/pr89093-2.c: New test.

--- gcc/config/arm/arm.c.jj	2019-04-13 17:20:07.353977370 +0200
+++ gcc/config/arm/arm.c	2019-04-15 19:50:31.386414421 +0200
@@ -30871,8 +30871,6 @@ arm_valid_target_attribute_rec (tree arg
 
   while ((q = strtok (argstr, ",")) != NULL)
     {
-      while (ISSPACE (*q)) ++q;
-
       argstr = NULL;
       if (!strcmp (q, "thumb"))
 	opts->x_target_flags |= MASK_THUMB;
--- gcc/testsuite/gcc.target/arm/pr89093-2.c.jj	2019-04-15 19:53:23.740608673 +0200
+++ gcc/testsuite/gcc.target/arm/pr89093-2.c	2019-04-15 19:52:29.841486100 +0200
@@ -0,0 +1,9 @@
+/* PR target/89093 */
+/* { dg-do compile } */
+
+__attribute__((target (" arm"))) void f1 (void) {} /* { dg-error "unknown target attribute or pragma ' arm'" } */
+__attribute__((target ("   thumb"))) void f2 (void) {} /* { dg-error "unknown target attribute or pragma '   thumb'" } */
+__attribute__((target ("arm,  thumb"))) void f3 (void) {} /* { dg-error "unknown target attribute or pragma '  thumb'" } */
+__attribute__((target ("thumb,  arm"))) void f4 (void) {} /* { dg-error "unknown target attribute or pragma '  arm'" } */
+#pragma GCC target ("    arm")	/* { dg-error "unknown target attribute or pragma '    arm'" } */
+void f5 (void) {}


	Jakub

[PATCH] Don't ignore leading whitespace in AArch64 target attribute/pragma (PR target/89093)

[Jakub Jelinek]

On Tue, Apr 16, 2019 at 07:50:35PM +0200, Jakub Jelinek wrote:
> On Fri, Apr 12, 2019 at 05:10:48PM +0100, Ramana Radhakrishnan wrote:
> > No, that's not right. we should get rid of this.
> 
> Here is a patch for that.
> 
> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?

And here is the same thing for aarch64.  Bootstrapped/regtested on
aarch64-linux, ok for trunk?

I think it is better not to accept any spaces in there, than accepting it
only at the beginning and after , but not e.g. at the end of before ,
like the trunk currently does, furthermore, e.g. x86 or ppc don't allow
spaces there.

2019-04-16  Jakub Jelinek  <jakub@redhat.com>

	PR target/89093
	* config/aarch64/aarch64.c (aarch64_process_one_target_attr): Don't skip
	whitespace at the start of target attribute string.

	* gcc.target/aarch64/pr89093.c: New test.
	* gcc.target/aarch64/pr63304_1.c: Remove space from target string.

--- gcc/config/aarch64/aarch64.c.jj	2019-04-11 10:26:22.907293129 +0200
+++ gcc/config/aarch64/aarch64.c	2019-04-15 19:59:55.784226278 +0200
@@ -12536,10 +12536,6 @@ aarch64_process_one_target_attr (char *a
   char *str_to_check = (char *) alloca (len + 1);
   strcpy (str_to_check, arg_str);
 
-  /* Skip leading whitespace.  */
-  while (*str_to_check == ' ' || *str_to_check == '\t')
-    str_to_check++;
-
   /* We have something like __attribute__ ((target ("+fp+nosimd"))).
      It is easier to detect and handle it explicitly here rather than going
      through the machinery for the rest of the target attributes in this
--- gcc/testsuite/gcc.target/aarch64/pr89093.c.jj	2019-04-15 20:02:25.456788897 +0200
+++ gcc/testsuite/gcc.target/aarch64/pr89093.c	2019-04-15 20:02:04.433131260 +0200
@@ -0,0 +1,7 @@
+/* PR target/89093 */
+/* { dg-do compile } */
+
+__attribute__((target ("  no-strict-align"))) void f1 (void) {} /* { dg-error "is not valid" } */
+__attribute__((target ("	general-regs-only"))) void f2 (void) {} /* { dg-error "is not valid" } */
+#pragma GCC target ("    general-regs-only")	/* { dg-error "is not valid" } */
+void f3 (void) {}
--- gcc/testsuite/gcc.target/aarch64/pr63304_1.c.jj	2017-09-13 16:22:19.795513580 +0200
+++ gcc/testsuite/gcc.target/aarch64/pr63304_1.c	2019-04-15 20:27:17.724847578 +0200
@@ -1,7 +1,7 @@
 /* { dg-do assemble } */
 /* { dg-options "-O1 --save-temps" } */
 #pragma GCC push_options
-#pragma GCC target ("+nothing+simd, cmodel=small")
+#pragma GCC target ("+nothing+simd,cmodel=small")
 
 int
 cal (double a)


	Jakub

Re: [PATCH] Don't ignore leading whitespace in ARM target attribute/pragma (PR target/89093)

[Kyrill Tkachov]

On 4/16/19 6:50 PM, Jakub Jelinek wrote:
> On Fri, Apr 12, 2019 at 05:10:48PM +0100, Ramana Radhakrishnan wrote:
> > No, that's not right. we should get rid of this.
>
> Here is a patch for that.
>
> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
>

Ok. I don't think anyone relies on this behaviour.

Thanks,

Kyrill


> 2019-04-16  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/89093
>         * config/arm/arm.c (arm_valid_target_attribute_rec): Don't skip
>         whitespace at the start of target attribute string.
>
>         * gcc.target/arm/pr89093-2.c: New test.
>
> --- gcc/config/arm/arm.c.jj     2019-04-13 17:20:07.353977370 +0200
> +++ gcc/config/arm/arm.c        2019-04-15 19:50:31.386414421 +0200
> @@ -30871,8 +30871,6 @@ arm_valid_target_attribute_rec (tree arg
>
>    while ((q = strtok (argstr, ",")) != NULL)
>      {
> -      while (ISSPACE (*q)) ++q;
> -
>        argstr = NULL;
>        if (!strcmp (q, "thumb"))
>          opts->x_target_flags |= MASK_THUMB;
> --- gcc/testsuite/gcc.target/arm/pr89093-2.c.jj 2019-04-15 
> 19:53:23.740608673 +0200
> +++ gcc/testsuite/gcc.target/arm/pr89093-2.c    2019-04-15 
> 19:52:29.841486100 +0200
> @@ -0,0 +1,9 @@
> +/* PR target/89093 */
> +/* { dg-do compile } */
> +
> +__attribute__((target (" arm"))) void f1 (void) {} /* { dg-error 
> "unknown target attribute or pragma ' arm'" } */
> +__attribute__((target ("   thumb"))) void f2 (void) {} /* { dg-error 
> "unknown target attribute or pragma '   thumb'" } */
> +__attribute__((target ("arm,  thumb"))) void f3 (void) {} /* { 
> dg-error "unknown target attribute or pragma '  thumb'" } */
> +__attribute__((target ("thumb,  arm"))) void f4 (void) {} /* { 
> dg-error "unknown target attribute or pragma '  arm'" } */
> +#pragma GCC target ("    arm") /* { dg-error "unknown target 
> attribute or pragma '    arm'" } */
> +void f5 (void) {}
>
>
>         Jakub

Re: [PATCH] Don't ignore leading whitespace in AArch64 target attribute/pragma (PR target/89093)

[Kyrill Tkachov]

HI Jakub,

On 4/16/19 7:32 PM, Jakub Jelinek wrote:
> On Tue, Apr 16, 2019 at 07:50:35PM +0200, Jakub Jelinek wrote:
> > On Fri, Apr 12, 2019 at 05:10:48PM +0100, Ramana Radhakrishnan wrote:
> > > No, that's not right. we should get rid of this.
> >
> > Here is a patch for that.
> >
> > Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
>
> And here is the same thing for aarch64. Bootstrapped/regtested on
> aarch64-linux, ok for trunk?


FWIW this looks ok to me implementation-wise (since I wrote that code a 
few years ago).

>
> I think it is better not to accept any spaces in there, than accepting it
> only at the beginning and after , but not e.g. at the end of before ,
> like the trunk currently does, furthermore, e.g. x86 or ppc don't allow
> spaces there.

Thinking about it a bit more, I think it's a good idea to disallow 
leading and trailing whitespaces.

But there could be a case for allowing whitespaces between separate 
target attributes.

Personally, I would find it more readable to have a space after a comma.

Similarly, spaces are allowed in the general attribute syntax, for 
example in our intrinsics header we have:

__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))

That said, distinguishing between the two classes of whitespace is 
probably more complexity than it's worth

and if other targets don't allow it then I won't let it block this patch.

Thanks,

Kyrill


>
> 2019-04-16  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/89093
>         * config/aarch64/aarch64.c (aarch64_process_one_target_attr): 
> Don't skip
>         whitespace at the start of target attribute string.
>
>         * gcc.target/aarch64/pr89093.c: New test.
>         * gcc.target/aarch64/pr63304_1.c: Remove space from target string.
>
> --- gcc/config/aarch64/aarch64.c.jj     2019-04-11 10:26:22.907293129 
> +0200
> +++ gcc/config/aarch64/aarch64.c        2019-04-15 19:59:55.784226278 
> +0200
> @@ -12536,10 +12536,6 @@ aarch64_process_one_target_attr (char *a
>    char *str_to_check = (char *) alloca (len + 1);
>    strcpy (str_to_check, arg_str);
>
> -  /* Skip leading whitespace.  */
> -  while (*str_to_check == ' ' || *str_to_check == '\t')
> -    str_to_check++;
> -
>    /* We have something like __attribute__ ((target ("+fp+nosimd"))).
>       It is easier to detect and handle it explicitly here rather than 
> going
>       through the machinery for the rest of the target attributes in this
> --- gcc/testsuite/gcc.target/aarch64/pr89093.c.jj 2019-04-15 
> 20:02:25.456788897 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr89093.c  2019-04-15 
> 20:02:04.433131260 +0200
> @@ -0,0 +1,7 @@
> +/* PR target/89093 */
> +/* { dg-do compile } */
> +
> +__attribute__((target ("  no-strict-align"))) void f1 (void) {} /* { 
> dg-error "is not valid" } */
> +__attribute__((target ("       general-regs-only"))) void f2 (void) 
> {} /* { dg-error "is not valid" } */
> +#pragma GCC target ("    general-regs-only")   /* { dg-error "is not 
> valid" } */
> +void f3 (void) {}
> --- gcc/testsuite/gcc.target/aarch64/pr63304_1.c.jj 2017-09-13 
> 16:22:19.795513580 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr63304_1.c 2019-04-15 
> 20:27:17.724847578 +0200
> @@ -1,7 +1,7 @@
>  /* { dg-do assemble } */
>  /* { dg-options "-O1 --save-temps" } */
>  #pragma GCC push_options
> -#pragma GCC target ("+nothing+simd, cmodel=small")
> +#pragma GCC target ("+nothing+simd,cmodel=small")
>
>  int
>  cal (double a)
>
>
>         Jakub

Re: [PATCH] Don't ignore leading whitespace in AArch64 target attribute/pragma (PR target/89093)

[Jakub Jelinek]

On Wed, Apr 17, 2019 at 08:59:08AM +0100, Kyrill Tkachov wrote:
> Similarly, spaces are allowed in the general attribute syntax, for example
> in our intrinsics header we have:
> 
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))

Well, that is how the C/C++ lexing works.  We also allow
__attribute__ ((__always_inline__		, __gnu_inline__

,

__artificial__
\f
))
etc.

The whitespace skipping in the target string handling allowed
target (" abc,  def")
but didn't allow
target ("abc , def")
or
target ("abc ,def")
etc.
IMHO either we shouldn't allow any whitespace anywhere, or allow it
everywhere (leading, trailing, before or after comma), but then for
consistency all targets should do that.
If one wants to do some whitespace, there is always an option to do
target ("abc,"
	"def")
or
target ("abc,"		"def")
or
#define C(x) #x
target (C(abc) 	","	C(def))
or whatever else one wants to use.

	Jakub

Patch ping (was Re: [PATCH] Don't ignore leading whitespace in AArch64 target attribute/pragma (PR target/89093))

[Jakub Jelinek]

Hi!

On Tue, Apr 16, 2019 at 08:32:50PM +0200, Jakub Jelinek wrote:
> 2019-04-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/89093
> 	* config/aarch64/aarch64.c (aarch64_process_one_target_attr): Don't skip
> 	whitespace at the start of target attribute string.
> 
> 	* gcc.target/aarch64/pr89093.c: New test.
> 	* gcc.target/aarch64/pr63304_1.c: Remove space from target string.

I'd like to ping this patch.
Thanks.

> --- gcc/config/aarch64/aarch64.c.jj	2019-04-11 10:26:22.907293129 +0200
> +++ gcc/config/aarch64/aarch64.c	2019-04-15 19:59:55.784226278 +0200
> @@ -12536,10 +12536,6 @@ aarch64_process_one_target_attr (char *a
>    char *str_to_check = (char *) alloca (len + 1);
>    strcpy (str_to_check, arg_str);
>  
> -  /* Skip leading whitespace.  */
> -  while (*str_to_check == ' ' || *str_to_check == '\t')
> -    str_to_check++;
> -
>    /* We have something like __attribute__ ((target ("+fp+nosimd"))).
>       It is easier to detect and handle it explicitly here rather than going
>       through the machinery for the rest of the target attributes in this
> --- gcc/testsuite/gcc.target/aarch64/pr89093.c.jj	2019-04-15 20:02:25.456788897 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr89093.c	2019-04-15 20:02:04.433131260 +0200
> @@ -0,0 +1,7 @@
> +/* PR target/89093 */
> +/* { dg-do compile } */
> +
> +__attribute__((target ("  no-strict-align"))) void f1 (void) {} /* { dg-error "is not valid" } */
> +__attribute__((target ("	general-regs-only"))) void f2 (void) {} /* { dg-error "is not valid" } */
> +#pragma GCC target ("    general-regs-only")	/* { dg-error "is not valid" } */
> +void f3 (void) {}
> --- gcc/testsuite/gcc.target/aarch64/pr63304_1.c.jj	2017-09-13 16:22:19.795513580 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr63304_1.c	2019-04-15 20:27:17.724847578 +0200
> @@ -1,7 +1,7 @@
>  /* { dg-do assemble } */
>  /* { dg-options "-O1 --save-temps" } */
>  #pragma GCC push_options
> -#pragma GCC target ("+nothing+simd, cmodel=small")
> +#pragma GCC target ("+nothing+simd,cmodel=small")
>  
>  int
>  cal (double a)

	Jakub

Re: [PATCH] Don't ignore leading whitespace in AArch64 target attribute/pragma (PR target/89093)

[Richard Earnshaw (lists)]

On 16/04/2019 19:32, Jakub Jelinek wrote:
> On Tue, Apr 16, 2019 at 07:50:35PM +0200, Jakub Jelinek wrote:
>> On Fri, Apr 12, 2019 at 05:10:48PM +0100, Ramana Radhakrishnan wrote:
>>> No, that's not right. we should get rid of this.
>>
>> Here is a patch for that.
>>
>> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
> 
> And here is the same thing for aarch64.  Bootstrapped/regtested on
> aarch64-linux, ok for trunk?
> 
> I think it is better not to accept any spaces in there, than accepting it
> only at the beginning and after , but not e.g. at the end of before ,
> like the trunk currently does, furthermore, e.g. x86 or ppc don't allow
> spaces there.
> 
> 2019-04-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/89093
> 	* config/aarch64/aarch64.c (aarch64_process_one_target_attr): Don't skip
> 	whitespace at the start of target attribute string.
> 
> 	* gcc.target/aarch64/pr89093.c: New test.
> 	* gcc.target/aarch64/pr63304_1.c: Remove space from target string.

OK.  I'll let you decide if you want this on gcc-9 as well :-)

R.

> 
> --- gcc/config/aarch64/aarch64.c.jj	2019-04-11 10:26:22.907293129 +0200
> +++ gcc/config/aarch64/aarch64.c	2019-04-15 19:59:55.784226278 +0200
> @@ -12536,10 +12536,6 @@ aarch64_process_one_target_attr (char *a
>    char *str_to_check = (char *) alloca (len + 1);
>    strcpy (str_to_check, arg_str);
>  
> -  /* Skip leading whitespace.  */
> -  while (*str_to_check == ' ' || *str_to_check == '\t')
> -    str_to_check++;
> -
>    /* We have something like __attribute__ ((target ("+fp+nosimd"))).
>       It is easier to detect and handle it explicitly here rather than going
>       through the machinery for the rest of the target attributes in this
> --- gcc/testsuite/gcc.target/aarch64/pr89093.c.jj	2019-04-15 20:02:25.456788897 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr89093.c	2019-04-15 20:02:04.433131260 +0200
> @@ -0,0 +1,7 @@
> +/* PR target/89093 */
> +/* { dg-do compile } */
> +
> +__attribute__((target ("  no-strict-align"))) void f1 (void) {} /* { dg-error "is not valid" } */
> +__attribute__((target ("	general-regs-only"))) void f2 (void) {} /* { dg-error "is not valid" } */
> +#pragma GCC target ("    general-regs-only")	/* { dg-error "is not valid" } */
> +void f3 (void) {}
> --- gcc/testsuite/gcc.target/aarch64/pr63304_1.c.jj	2017-09-13 16:22:19.795513580 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr63304_1.c	2019-04-15 20:27:17.724847578 +0200
> @@ -1,7 +1,7 @@
>  /* { dg-do assemble } */
>  /* { dg-options "-O1 --save-temps" } */
>  #pragma GCC push_options
> -#pragma GCC target ("+nothing+simd, cmodel=small")
> +#pragma GCC target ("+nothing+simd,cmodel=small")
>  
>  int
>  cal (double a)
> 
> 
> 	Jakub
>