diff mbox series

[v1] Internal-fn: Handle vector bool type for type strict match mode [PR116103]

Message ID 20240729075724.3722156-1-pan2.li@intel.com
State New
Headers show
Series [v1] Internal-fn: Handle vector bool type for type strict match mode [PR116103] | expand

Commit Message

Li, Pan2 July 29, 2024, 7:57 a.m. UTC 
From: Pan Li <pan2.li@intel.com>

For some target like target=amdgcn-amdhsa,  we need to take care of
vector bool types prior to general vector mode types.  Or we may have
the asm check failure as below.

gcc.target/gcn/cond_smax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
gcc.target/gcn/cond_smin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
gcc.target/gcn/cond_umax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
gcc.target/gcn/cond_umin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
gcc.dg/tree-ssa/loop-bound-2.c scan-tree-dump-not ivopts "zero if "

The below test suites are passed for this patch.
1. The rv64gcv fully regression tests.
2. The x86 bootstrap tests.
3. The x86 fully regression tests.
4. The amdgcn test case as above.

gcc/ChangeLog:

	* internal-fn.cc (type_strictly_matches_mode_p): Add handling
	for vector bool type.

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/internal-fn.cc | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Richard Biener July 29, 2024, 8:44 a.m. UTC | #1 
On Mon, Jul 29, 2024 at 9:57 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> For some target like target=amdgcn-amdhsa,  we need to take care of
> vector bool types prior to general vector mode types.  Or we may have
> the asm check failure as below.
>
> gcc.target/gcn/cond_smax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_smin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_umax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.target/gcn/cond_umin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.dg/tree-ssa/loop-bound-2.c scan-tree-dump-not ivopts "zero if "
>
> The below test suites are passed for this patch.
> 1. The rv64gcv fully regression tests.
> 2. The x86 bootstrap tests.
> 3. The x86 fully regression tests.
> 4. The amdgcn test case as above.

OK.

Richard.

> gcc/ChangeLog:
>
>         * internal-fn.cc (type_strictly_matches_mode_p): Add handling
>         for vector bool type.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/internal-fn.cc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 8a2e07f2f96..086c8be398a 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -4171,6 +4171,12 @@ direct_internal_fn_optab (internal_fn fn)
>  static bool
>  type_strictly_matches_mode_p (const_tree type)
>  {
> +  /* For target=amdgcn-amdhsa,  we need to take care of vector bool types.
> +     More details see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116103.  */
> +  if (VECTOR_BOOLEAN_TYPE_P (type) && SCALAR_INT_MODE_P (TYPE_MODE (type))
> +    && TYPE_PRECISION (TREE_TYPE (type)) == 1)
> +    return true;
> +
>    if (VECTOR_TYPE_P (type))
>      return VECTOR_MODE_P (TYPE_MODE (type));
>
> --
> 2.34.1
>
Li, Pan2 July 29, 2024, 12:24 p.m. UTC | #2 
> OK.

Thanks Richard, will wait the confirmation from Thomas in case I missed some more failed cases.

Pan

-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com> 
Sent: Monday, July 29, 2024 4:44 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; tamar.christina@arm.com; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com
Subject: Re: [PATCH v1] Internal-fn: Handle vector bool type for type strict match mode [PR116103]

On Mon, Jul 29, 2024 at 9:57 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> For some target like target=amdgcn-amdhsa,  we need to take care of
> vector bool types prior to general vector mode types.  Or we may have
> the asm check failure as below.
>
> gcc.target/gcn/cond_smax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_smin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_umax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.target/gcn/cond_umin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.dg/tree-ssa/loop-bound-2.c scan-tree-dump-not ivopts "zero if "
>
> The below test suites are passed for this patch.
> 1. The rv64gcv fully regression tests.
> 2. The x86 bootstrap tests.
> 3. The x86 fully regression tests.
> 4. The amdgcn test case as above.

OK.

Richard.

> gcc/ChangeLog:
>
>         * internal-fn.cc (type_strictly_matches_mode_p): Add handling
>         for vector bool type.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/internal-fn.cc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 8a2e07f2f96..086c8be398a 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -4171,6 +4171,12 @@ direct_internal_fn_optab (internal_fn fn)
>  static bool
>  type_strictly_matches_mode_p (const_tree type)
>  {
> +  /* For target=amdgcn-amdhsa,  we need to take care of vector bool types.
> +     More details see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116103.  */
> +  if (VECTOR_BOOLEAN_TYPE_P (type) && SCALAR_INT_MODE_P (TYPE_MODE (type))
> +    && TYPE_PRECISION (TREE_TYPE (type)) == 1)
> +    return true;
> +
>    if (VECTOR_TYPE_P (type))
>      return VECTOR_MODE_P (TYPE_MODE (type));
>
> --
> 2.34.1
>
Richard Sandiford July 29, 2024, 4:08 p.m. UTC | #3 
pan2.li@intel.com writes:
> From: Pan Li <pan2.li@intel.com>
>
> For some target like target=amdgcn-amdhsa,  we need to take care of
> vector bool types prior to general vector mode types.  Or we may have
> the asm check failure as below.
>
> gcc.target/gcn/cond_smax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_smin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_umax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.target/gcn/cond_umin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.dg/tree-ssa/loop-bound-2.c scan-tree-dump-not ivopts "zero if "
>
> The below test suites are passed for this patch.
> 1. The rv64gcv fully regression tests.
> 2. The x86 bootstrap tests.
> 3. The x86 fully regression tests.
> 4. The amdgcn test case as above.
>
> gcc/ChangeLog:
>
> 	* internal-fn.cc (type_strictly_matches_mode_p): Add handling
> 	for vector bool type.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/internal-fn.cc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 8a2e07f2f96..086c8be398a 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -4171,6 +4171,12 @@ direct_internal_fn_optab (internal_fn fn)
>  static bool
>  type_strictly_matches_mode_p (const_tree type)
>  {
> +  /* For target=amdgcn-amdhsa,  we need to take care of vector bool types.
> +     More details see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116103.  */
> +  if (VECTOR_BOOLEAN_TYPE_P (type) && SCALAR_INT_MODE_P (TYPE_MODE (type))
> +    && TYPE_PRECISION (TREE_TYPE (type)) == 1)

Sorry for the formatting nits, but I think this should be:

  if (VECTOR_BOOLEAN_TYPE_P (type)
      && SCALAR_INT_MODE_P (TYPE_MODE (type))
      && TYPE_PRECISION (TREE_TYPE (type)) == 1)

(one condition per line, indented below "VECTOR").

But I think the comment should give the underlying reason, rather than
treat it as a target oddity.  Maybe something like:

  /* Masked vector operations have both vector data operands and
     vector boolean operands.  The vector data operands are expected
     to have a vector mode, but the vector boolean operands can be
     an integer mode rather than a vector mode, depending on how
     TARGET_VECTORIZE_GET_MASK_MODE is defined.  */

Thanks,
Richard

> +    return true;
> +
>    if (VECTOR_TYPE_P (type))
>      return VECTOR_MODE_P (TYPE_MODE (type));
Li, Pan2 July 30, 2024, 3:10 a.m. UTC | #4 
Thanks Richard S for comments, updated in v2.

https://gcc.gnu.org/pipermail/gcc-patches/2024-July/658637.html

Pan

-----Original Message-----
From: Richard Sandiford <richard.sandiford@arm.com> 
Sent: Tuesday, July 30, 2024 12:09 AM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; richard.guenther@gmail.com; tamar.christina@arm.com; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com
Subject: Re: [PATCH v1] Internal-fn: Handle vector bool type for type strict match mode [PR116103]

pan2.li@intel.com writes:
> From: Pan Li <pan2.li@intel.com>
>
> For some target like target=amdgcn-amdhsa,  we need to take care of
> vector bool types prior to general vector mode types.  Or we may have
> the asm check failure as below.
>
> gcc.target/gcn/cond_smax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_smin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 80
> gcc.target/gcn/cond_umax_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.target/gcn/cond_umin_1.c scan-assembler-times \\tv_cmp_gt_i32\\tvcc, s[0-9]+, v[0-9]+ 56
> gcc.dg/tree-ssa/loop-bound-2.c scan-tree-dump-not ivopts "zero if "
>
> The below test suites are passed for this patch.
> 1. The rv64gcv fully regression tests.
> 2. The x86 bootstrap tests.
> 3. The x86 fully regression tests.
> 4. The amdgcn test case as above.
>
> gcc/ChangeLog:
>
> 	* internal-fn.cc (type_strictly_matches_mode_p): Add handling
> 	for vector bool type.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/internal-fn.cc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 8a2e07f2f96..086c8be398a 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -4171,6 +4171,12 @@ direct_internal_fn_optab (internal_fn fn)
>  static bool
>  type_strictly_matches_mode_p (const_tree type)
>  {
> +  /* For target=amdgcn-amdhsa,  we need to take care of vector bool types.
> +     More details see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116103.  */
> +  if (VECTOR_BOOLEAN_TYPE_P (type) && SCALAR_INT_MODE_P (TYPE_MODE (type))
> +    && TYPE_PRECISION (TREE_TYPE (type)) == 1)

Sorry for the formatting nits, but I think this should be:

  if (VECTOR_BOOLEAN_TYPE_P (type)
      && SCALAR_INT_MODE_P (TYPE_MODE (type))
      && TYPE_PRECISION (TREE_TYPE (type)) == 1)

(one condition per line, indented below "VECTOR").

But I think the comment should give the underlying reason, rather than
treat it as a target oddity.  Maybe something like:

  /* Masked vector operations have both vector data operands and
     vector boolean operands.  The vector data operands are expected
     to have a vector mode, but the vector boolean operands can be
     an integer mode rather than a vector mode, depending on how
     TARGET_VECTORIZE_GET_MASK_MODE is defined.  */

Thanks,
Richard

> +    return true;
> +
>    if (VECTOR_TYPE_P (type))
>      return VECTOR_MODE_P (TYPE_MODE (type));
diff mbox series

Patch

diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
index 8a2e07f2f96..086c8be398a 100644
--- a/gcc/internal-fn.cc
+++ b/gcc/internal-fn.cc
@@ -4171,6 +4171,12 @@  direct_internal_fn_optab (internal_fn fn)
 static bool
 type_strictly_matches_mode_p (const_tree type)
 {
+  /* For target=amdgcn-amdhsa,  we need to take care of vector bool types.
+     More details see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116103.  */
+  if (VECTOR_BOOLEAN_TYPE_P (type) && SCALAR_INT_MODE_P (TYPE_MODE (type))
+    && TYPE_PRECISION (TREE_TYPE (type)) == 1)
+    return true;
+
   if (VECTOR_TYPE_P (type))
     return VECTOR_MODE_P (TYPE_MODE (type));