diff mbox series

Fix arm_neon.h #pragma GCC target syntax (PR target/88734)

Message ID 20190117134732.GJ30353@tucnak
State New
Headers show
Series Fix arm_neon.h #pragma GCC target syntax (PR target/88734) | expand

Commit Message

Jakub Jelinek Jan. 17, 2019, 1:47 p.m. UTC
Hi!

arm_neon.h on both targets contained a couple of spots with invalid
#pragma GCC target syntax.  This doesn't result in errors, just warnings and
those warnings are surpressed in system headers, so are visible with
-Wsystem-headers only.  Anyway, the end result was that these pragmas were
ignored, when they meant to be there.

The following patch fixes it.  Also, on aarch64 the sha3 intrinsics were
wrapped with arch=armv8.2-a+crypto rather than arch=armv8.2-a+sha3, but
because of the invalid syntax it wasn't covered in the testsuite.

Without the patch, besides -Wsystem-headers warnings on it, if somebody
attempts to use those intrinsics in code compiled with target options that
do not include the necessary ISA features, one will get ICEs rather than
errors.

Bootstrapped/regtested on aarch64-linux, ok for trunk?

Note, I haven't included a testcase, as I'm not familiar enough with
gcc.target/aarch64/ test style, but a test would be roughly include the
testcase from the PR, compile it with -march=something that doesn't include
the needed ISA options, probably have a dg-skip-if if somebody overrides it
from the --target_board and make sure it emits a dg-error message rather
than ICE.

2019-01-17  Jakub Jelinek  <jakub@redhat.com>

	PR target/88734
	* config/arm/arm_neon.h: Fix #pragma GCC target syntax - replace
	(("..."))) with ("...").
	* config/aarch64/arm_neon.h: Likewise.  Use arch=armv8.2-a+sha3
	instead of arch=armv8.2-a+crypto for vsha512hq_u64 etc. intrinsics.


	Jakub

Comments

James Greenhalgh Jan. 17, 2019, 11:14 p.m. UTC | #1
On Thu, Jan 17, 2019 at 07:47:32AM -0600, Jakub Jelinek wrote:
> Hi!
> 
> arm_neon.h on both targets contained a couple of spots with invalid
> #pragma GCC target syntax.  This doesn't result in errors, just warnings and
> those warnings are surpressed in system headers, so are visible with
> -Wsystem-headers only.  Anyway, the end result was that these pragmas were
> ignored, when they meant to be there.
> 
> The following patch fixes it.  Also, on aarch64 the sha3 intrinsics were
> wrapped with arch=armv8.2-a+crypto rather than arch=armv8.2-a+sha3, but
> because of the invalid syntax it wasn't covered in the testsuite.
> 
> Without the patch, besides -Wsystem-headers warnings on it, if somebody
> attempts to use those intrinsics in code compiled with target options that
> do not include the necessary ISA features, one will get ICEs rather than
> errors.
> 
> Bootstrapped/regtested on aarch64-linux, ok for trunk?
> 
> Note, I haven't included a testcase, as I'm not familiar enough with
> gcc.target/aarch64/ test style, but a test would be roughly include the
> testcase from the PR, compile it with -march=something that doesn't include
> the needed ISA options, probably have a dg-skip-if if somebody overrides it
> from the --target_board and make sure it emits a dg-error message rather
> than ICE.

AArch64 parts of this are OK by me. Thanks for the fix.

James

> 
> 2019-01-17  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/88734
> 	* config/arm/arm_neon.h: Fix #pragma GCC target syntax - replace
> 	(("..."))) with ("...").
> 	* config/aarch64/arm_neon.h: Likewise.  Use arch=armv8.2-a+sha3
> 	instead of arch=armv8.2-a+crypto for vsha512hq_u64 etc. intrinsics.
Kyrill Tkachov Jan. 18, 2019, 9:11 a.m. UTC | #2
Hi Jakub,

On 17/01/19 13:47, Jakub Jelinek wrote:
> Hi!
>
> arm_neon.h on both targets contained a couple of spots with invalid
> #pragma GCC target syntax.  This doesn't result in errors, just warnings and
> those warnings are surpressed in system headers, so are visible with
> -Wsystem-headers only.  Anyway, the end result was that these pragmas were
> ignored, when they meant to be there.
>
> The following patch fixes it.  Also, on aarch64 the sha3 intrinsics were
> wrapped with arch=armv8.2-a+crypto rather than arch=armv8.2-a+sha3, but
> because of the invalid syntax it wasn't covered in the testsuite.
>
> Without the patch, besides -Wsystem-headers warnings on it, if somebody
> attempts to use those intrinsics in code compiled with target options that
> do not include the necessary ISA features, one will get ICEs rather than
> errors.
>
> Bootstrapped/regtested on aarch64-linux, ok for trunk?
>
> Note, I haven't included a testcase, as I'm not familiar enough with
> gcc.target/aarch64/ test style, but a test would be roughly include the
> testcase from the PR, compile it with -march=something that doesn't include
> the needed ISA options, probably have a dg-skip-if if somebody overrides it
> from the --target_board and make sure it emits a dg-error message rather
> than ICE.
>

The arm parts are ok.
Thanks,
Kyrill

> 2019-01-17  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/88734
>         * config/arm/arm_neon.h: Fix #pragma GCC target syntax - replace
>         (("..."))) with ("...").
>         * config/aarch64/arm_neon.h: Likewise.  Use arch=armv8.2-a+sha3
>         instead of arch=armv8.2-a+crypto for vsha512hq_u64 etc. intrinsics.
>
> --- gcc/config/arm/arm_neon.h.jj        2019-01-10 11:43:20.100283845 +0100
> +++ gcc/config/arm/arm_neon.h   2019-01-16 17:28:32.830228005 +0100
> @@ -18310,12 +18310,12 @@ vfmlsl_laneq_high_u32 (float32x2_t __r,
>  /* AdvSIMD Complex numbers intrinsics.  */
>  #if __ARM_ARCH >= 8
>  #pragma GCC push_options
> -#pragma GCC target(("arch=armv8.3-a"))
> +#pragma GCC target ("arch=armv8.3-a")
>
>
>  #if defined (__ARM_FP16_FORMAT_IEEE) || defined (__ARM_FP16_FORMAT_ALTERNATIVE)
>  #pragma GCC push_options
> -#pragma GCC target(("+fp16"))
> +#pragma GCC target ("+fp16")
>  __extension__ extern __inline float16x4_t
>  __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
>  vcadd_rot90_f16 (float16x4_t __a, float16x4_t __b)
> --- gcc/config/aarch64/arm_neon.h.jj    2019-01-10 11:43:18.620308158 +0100
> +++ gcc/config/aarch64/arm_neon.h       2019-01-16 17:27:30.170252504 +0100
> @@ -33070,7 +33070,7 @@ vdotq_laneq_s32 (int32x4_t __r, int8x16_
>  #pragma GCC pop_options
>
>  #pragma GCC push_options
> -#pragma GCC target(("arch=armv8.2-a+sm4"))
> +#pragma GCC target ("arch=armv8.2-a+sm4")
>
>  __extension__ extern __inline uint32x4_t
>  __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> @@ -33137,7 +33137,7 @@ vsm4ekeyq_u32 (uint32x4_t __a, uint32x4_
>  #pragma GCC pop_options
>
>  #pragma GCC push_options
> -#pragma GCC target(("arch=armv8.2-a+crypto"))
> +#pragma GCC target ("arch=armv8.2-a+sha3")
>
>  __extension__ extern __inline uint64x2_t
>  __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> @@ -33299,10 +33299,10 @@ vbcaxq_s64 (int64x2_t __a, int64x2_t __b
>  /* AdvSIMD Complex numbers intrinsics.  */
>
>  #pragma GCC push_options
> -#pragma GCC target(("arch=armv8.3-a"))
> +#pragma GCC target ("arch=armv8.3-a")
>
>  #pragma GCC push_options
> -#pragma GCC target(("+fp16"))
> +#pragma GCC target ("+fp16")
>  __extension__ extern __inline float16x4_t
>  __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
>  vcadd_rot90_f16 (float16x4_t __a, float16x4_t __b)
> @@ -33773,7 +33773,7 @@ vcmlaq_rot270_laneq_f32 (float32x4_t __r
>  #pragma GCC pop_options
>
>  #pragma GCC push_options
> -#pragma GCC target(("arch=armv8.2-a+fp16fml"))
> +#pragma GCC target ("arch=armv8.2-a+fp16fml")
>
>  __extension__ extern __inline float32x2_t
>  __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
>
>         Jakub
diff mbox series

Patch

--- gcc/config/arm/arm_neon.h.jj	2019-01-10 11:43:20.100283845 +0100
+++ gcc/config/arm/arm_neon.h	2019-01-16 17:28:32.830228005 +0100
@@ -18310,12 +18310,12 @@  vfmlsl_laneq_high_u32 (float32x2_t __r,
 /* AdvSIMD Complex numbers intrinsics.  */
 #if __ARM_ARCH >= 8
 #pragma GCC push_options
-#pragma GCC target(("arch=armv8.3-a"))
+#pragma GCC target ("arch=armv8.3-a")
 
 
 #if defined (__ARM_FP16_FORMAT_IEEE) || defined (__ARM_FP16_FORMAT_ALTERNATIVE)
 #pragma GCC push_options
-#pragma GCC target(("+fp16"))
+#pragma GCC target ("+fp16")
 __extension__ extern __inline float16x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vcadd_rot90_f16 (float16x4_t __a, float16x4_t __b)
--- gcc/config/aarch64/arm_neon.h.jj	2019-01-10 11:43:18.620308158 +0100
+++ gcc/config/aarch64/arm_neon.h	2019-01-16 17:27:30.170252504 +0100
@@ -33070,7 +33070,7 @@  vdotq_laneq_s32 (int32x4_t __r, int8x16_
 #pragma GCC pop_options
 
 #pragma GCC push_options
-#pragma GCC target(("arch=armv8.2-a+sm4"))
+#pragma GCC target ("arch=armv8.2-a+sm4")
 
 __extension__ extern __inline uint32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
@@ -33137,7 +33137,7 @@  vsm4ekeyq_u32 (uint32x4_t __a, uint32x4_
 #pragma GCC pop_options
 
 #pragma GCC push_options
-#pragma GCC target(("arch=armv8.2-a+crypto"))
+#pragma GCC target ("arch=armv8.2-a+sha3")
 
 __extension__ extern __inline uint64x2_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
@@ -33299,10 +33299,10 @@  vbcaxq_s64 (int64x2_t __a, int64x2_t __b
 /* AdvSIMD Complex numbers intrinsics.  */
 
 #pragma GCC push_options
-#pragma GCC target(("arch=armv8.3-a"))
+#pragma GCC target ("arch=armv8.3-a")
 
 #pragma GCC push_options
-#pragma GCC target(("+fp16"))
+#pragma GCC target ("+fp16")
 __extension__ extern __inline float16x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 vcadd_rot90_f16 (float16x4_t __a, float16x4_t __b)
@@ -33773,7 +33773,7 @@  vcmlaq_rot270_laneq_f32 (float32x4_t __r
 #pragma GCC pop_options
 
 #pragma GCC push_options
-#pragma GCC target(("arch=armv8.2-a+fp16fml"))
+#pragma GCC target ("arch=armv8.2-a+fp16fml")
 
 __extension__ extern __inline float32x2_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))