[PATCH] Add missing avx512dqintrin.h _mm_mask_fpclass_s[sd]_mask (PR target/89803)

[Jakub Jelinek]

On Tue, Jun 04, 2019 at 03:38:08PM +0800, Hongtao Liu wrote:
> --- gcc/ChangeLog	(revision 271853)
> +++ gcc/ChangeLog	(working copy)
> @@ -4706,6 +4706,26 @@
>  	reprocessing.  Always call df_analyze before fixing up debug bind
>  	insns.
>  
> +2019-03-24 Hongtao Liu	<hongtao.liu@intel.com>

name should be separated from date and email by 2 spaces on each side,
you have just one space before and a tab after.

> +
> +	PR target/89803
> +	* config/i386/avx512dqintrin.h
> +	(_mm_mask_fpclass_ss_mask,_mm_mask_fpclass_sd_mask):
> +	New intrinsics.

There should be space after comma, and a line break should be there
only when it will not fit, so:

+	* config/i386/avx512dqintrin.h (_mm_mask_fpclass_ss_mask,
+	_mm_mask_fpclass_sd_mask): New intrinsics.

> +	(_mm_fpclass_ss_mask,_mm_fpclass_sd_mask):
> +	Modified, use new builtins.

Similarly.

> +	* config/i386/i386-builtin.def
> +	(__builtin_ia32_fpclassss_mask, _builtin_ia32_fpclasssd_mask):
> +	New builtins.

Again.

> +	(__builtin_ia32_fpclassss, _builtin_ia32_fpclasssd): Deleted.
> +	* config/i386/i386-builtin-types.def:
> +	Delete relate types.

You should say what exactly you've deleted, so

+	* config/i386/i386-builtin-types.def (QI_FTYPE_V2DF_INT,
+	QI_FTYPE_V4SF_INT): Remove.

> +	* config/i386/i386-expand.c:
> +	Ditto.

Mention what you've changed, so

+	* config/i386/i386-expand.c (ix86_expand_args_builtin): Remove
+	QI_FTYPE_V2DF_INT and QI_FTYPE_V4SF_INT cases.

> +	* config/i386/sse.md
> +	(define_insn "avx512dq_vmfpclass<mode><mask_scalar_merge_name>):
> +	Modified with mask.

That is not what you've done.

+	* config/i386/sse.md (avx512dq_vmfpclass<mode>): Rename to ...
+	(avx512dq_vmfpclass<mode><mask_scalar_merge_name>): ... this.  Add
+	<mask_scalar_merge_operand3> to insn template.

> --- gcc/config/i386/avx512dqintrin.h	(revision 271853)
> +++ gcc/config/i386/avx512dqintrin.h	(working copy)
> @@ -1362,7 +1362,7 @@
>  __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
>  _mm_fpclass_ss_mask (__m128 __A, const int __imm)
>  {
> -  return (__mmask8) __builtin_ia32_fpclassss ((__v4sf) __A, __imm);
> +  return (__mmask8) __builtin_ia32_fpclassss_mask ((__v4sf) __A, __imm, -1);

Most other avx512*.h code uses explicit (__mmaskN) -1 instead of just -1, so
perhaps for consistency use:
+  return (__mmask8) __builtin_ia32_fpclassss_mask ((__v4sf) __A, __imm,
+						    (_mmask8) -1);
?

>  }
>  
>  extern __inline __mmask8
> @@ -1369,9 +1369,23 @@
>  __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
>  _mm_fpclass_sd_mask (__m128d __A, const int __imm)
>  {
> -  return (__mmask8) __builtin_ia32_fpclasssd ((__v2df) __A, __imm);
> +  return (__mmask8) __builtin_ia32_fpclasssd_mask ((__v2df) __A, __imm, -1);
>  }

Likewise.

>  #define _mm_fpclass_ss_mask(X, C)						\
> -  ((__mmask8) __builtin_ia32_fpclassss ((__v4sf) (__m128) (X), (int) (C)))  \
> +  ((__mmask8) __builtin_ia32_fpclassss_mask ((__v4sf) (__m128) (X), (int) (C), (__mmask8) (-1))) \
>  
>  #define _mm_fpclass_sd_mask(X, C)						\
> -  ((__mmask8) __builtin_ia32_fpclasssd ((__v2df) (__m128d) (X), (int) (C))) \
> +  ((__mmask8) __builtin_ia32_fpclasssd_mask ((__v2df) (__m128d) (X), (int) (C), (__mmask8) (-1))) \
>  
> +#define _mm_mask_fpclass_ss_mask(X, C, U)					\
> +  ((__mmask8) __builtin_ia32_fpclassss_mask ((__v4sf) (__m128) (X), (int) (C), (__mmask8) (U)))
> +
> +#define _mm_mask_fpclass_sd_mask(X, C, U)					\
> +  ((__mmask8) __builtin_ia32_fpclasssd_mask ((__v2df) (__m128d) (X), (int) (C), (__mmask8) (U)))

Too long lines.

> +2019-03-24 Hongtao Liu <hongtao.liu@intel.com>
> +
> +	PR target/89803
> +	* gcc.target/i386/avx-1.c
> +	(__builtin_ia32_fpclasss[sd]): Replaced with builtin_ia32_fpclasss[sd]_mask.
> +	* gcc.target/i386/sse-13.c:
> +	(__builtin_ia32_fpclasss[sd]): Likewise.
> +	* gcc.target/i386/sse-23.c
> +	(__builtin_ia32_fpclasss[sd]): Likewise.

Similar problems in this ChangeLog as in gcc/ChangeLog, you don't want a
linebreak after the filename if the function name can fit in, too long line
too, sse-13.c has an extra : after it and I believe we don't allow wildcards
in the function names between
()s, so it should be:
+	* gcc.target/i386/avx-1.c (__builtin_ia32_fpclassss,
+	__builtin_ia32_fpclasssd): Remove.
+	(__builtin_ia32_fpclassss_mask, __builtin_ia32_fpclasssd_mask): Define.
etc.

	Jakub