[PATCH] vect: Use vect representative statement instead of original in patch recog [PR115060]

[PATCH] vect: Use vect representative statement instead of original in patch recog [PR115060]

[Feng Xue OS]

Some utility functions (such as vect_look_through_possible_promotion) that are
to find out certain kind of direct or indirect definition SSA for a value, may
return the original one of the SSA, not its pattern representative SSA, even
pattern is involved. For example,
    
   a = (T1) patt_b;
   patt_b = (T2) c;        // b = ...
   patt_c = not-a-cast;    // c = ...
    
Given 'a', the mentioned function will return 'c', instead of 'patt_c'. This
subtlety would make some pattern recog code that is unaware of it mis-use the
original instead of the new pattern statement, which is inconsistent wth
processing logic of the pattern formation pass. This patch corrects the issue
by forcing another utility function (vect_get_internal_def) return the pattern
statement information to caller by default.

Regression test on x86-64 and aarch64.

Feng
--
gcc/
	PR tree-optimization/115060
	* tree-vect-patterns.h (vect_get_internal_def): Add a new parameter
	for_vectorize.
	(vect_widened_op_tree): Call vect_get_internal_def instead of look_def
	to get statement information.
	(vect_recog_widen_abd_pattern): No need to call vect_stmt_to_vectorize.
---
 gcc/tree-vect-patterns.cc | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
index a313dc64643..fa35bf26372 100644
--- a/gcc/tree-vect-patterns.cc
+++ b/gcc/tree-vect-patterns.cc
@@ -258,15 +258,21 @@ vect_element_precision (unsigned int precision)
 }
 
 /* If OP is defined by a statement that's being considered for vectorization,
-   return information about that statement, otherwise return NULL.  */
+   return information about that statement, otherwise return NULL.
+   FOR_VECTORIZE is used to specify whether original or vectorization
+   representative (if have) statement information is returned.  */
 
 static stmt_vec_info
-vect_get_internal_def (vec_info *vinfo, tree op)
+vect_get_internal_def (vec_info *vinfo, tree op, bool for_vectorize = true)
 {
   stmt_vec_info def_stmt_info = vinfo->lookup_def (op);
   if (def_stmt_info
       && STMT_VINFO_DEF_TYPE (def_stmt_info) == vect_internal_def)
-    return def_stmt_info;
+    {
+      if (for_vectorize)
+	def_stmt_info = vect_stmt_to_vectorize (def_stmt_info);
+      return def_stmt_info;
+    }
   return NULL;
 }
 
@@ -655,7 +661,8 @@ vect_widened_op_tree (vec_info *vinfo, stmt_vec_info stmt_info, tree_code code,
 
 	      /* Recursively process the definition of the operand.  */
 	      stmt_vec_info def_stmt_info
-		= vinfo->lookup_def (this_unprom->op);
+		= vect_get_internal_def (vinfo, this_unprom->op);
+
 	      nops = vect_widened_op_tree (vinfo, def_stmt_info, code,
 					   widened_code, shift_p, max_nops,
 					   this_unprom, common_type,
@@ -1739,7 +1746,6 @@ vect_recog_widen_abd_pattern (vec_info *vinfo, stmt_vec_info stmt_vinfo,
   if (!abd_pattern_vinfo)
     return NULL;
 
-  abd_pattern_vinfo = vect_stmt_to_vectorize (abd_pattern_vinfo);
   gcall *abd_stmt = dyn_cast <gcall *> (STMT_VINFO_STMT (abd_pattern_vinfo));
   if (!abd_stmt

Re: [PATCH] vect: Use vect representative statement instead of original in patch recog [PR115060]

[Richard Biener]

On Sat, May 25, 2024 at 4:45 PM Feng Xue OS <fxue@os.amperecomputing.com> wrote:
>
> Some utility functions (such as vect_look_through_possible_promotion) that are
> to find out certain kind of direct or indirect definition SSA for a value, may
> return the original one of the SSA, not its pattern representative SSA, even
> pattern is involved. For example,
>
>    a = (T1) patt_b;
>    patt_b = (T2) c;        // b = ...
>    patt_c = not-a-cast;    // c = ...
>
> Given 'a', the mentioned function will return 'c', instead of 'patt_c'. This
> subtlety would make some pattern recog code that is unaware of it mis-use the
> original instead of the new pattern statement, which is inconsistent wth
> processing logic of the pattern formation pass. This patch corrects the issue
> by forcing another utility function (vect_get_internal_def) return the pattern
> statement information to caller by default.
>
> Regression test on x86-64 and aarch64.
>
> Feng
> --
> gcc/
>         PR tree-optimization/115060
>         * tree-vect-patterns.h (vect_get_internal_def): Add a new parameter
>         for_vectorize.
>         (vect_widened_op_tree): Call vect_get_internal_def instead of look_def
>         to get statement information.
>         (vect_recog_widen_abd_pattern): No need to call vect_stmt_to_vectorize.
> ---
>  gcc/tree-vect-patterns.cc | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
> index a313dc64643..fa35bf26372 100644
> --- a/gcc/tree-vect-patterns.cc
> +++ b/gcc/tree-vect-patterns.cc
> @@ -258,15 +258,21 @@ vect_element_precision (unsigned int precision)
>  }
>
>  /* If OP is defined by a statement that's being considered for vectorization,
> -   return information about that statement, otherwise return NULL.  */
> +   return information about that statement, otherwise return NULL.
> +   FOR_VECTORIZE is used to specify whether original or vectorization
> +   representative (if have) statement information is returned.  */
>
>  static stmt_vec_info
> -vect_get_internal_def (vec_info *vinfo, tree op)
> +vect_get_internal_def (vec_info *vinfo, tree op, bool for_vectorize = true)

I'm probably blind - but you nowhere pass 'false' and I think returning the
pattern stmt is the correct behavior always.

OK with omitting the new parameter.

>  {
>    stmt_vec_info def_stmt_info = vinfo->lookup_def (op);
>    if (def_stmt_info
>        && STMT_VINFO_DEF_TYPE (def_stmt_info) == vect_internal_def)
> -    return def_stmt_info;
> +    {
> +      if (for_vectorize)
> +       def_stmt_info = vect_stmt_to_vectorize (def_stmt_info);
> +      return def_stmt_info;
> +    }
>    return NULL;
>  }
>
> @@ -655,7 +661,8 @@ vect_widened_op_tree (vec_info *vinfo, stmt_vec_info stmt_info, tree_code code,
>
>               /* Recursively process the definition of the operand.  */
>               stmt_vec_info def_stmt_info
> -               = vinfo->lookup_def (this_unprom->op);
> +               = vect_get_internal_def (vinfo, this_unprom->op);
> +
>               nops = vect_widened_op_tree (vinfo, def_stmt_info, code,
>                                            widened_code, shift_p, max_nops,
>                                            this_unprom, common_type,
> @@ -1739,7 +1746,6 @@ vect_recog_widen_abd_pattern (vec_info *vinfo, stmt_vec_info stmt_vinfo,
>    if (!abd_pattern_vinfo)
>      return NULL;
>
> -  abd_pattern_vinfo = vect_stmt_to_vectorize (abd_pattern_vinfo);
>    gcall *abd_stmt = dyn_cast <gcall *> (STMT_VINFO_STMT (abd_pattern_vinfo));
>    if (!abd_stmt

Re: [PATCH] vect: Use vect representative statement instead of original in patch recog [PR115060]

[Feng Xue OS]

Changed as the comments.

Thanks,
Feng

________________________________________
From: Richard Biener <richard.guenther@gmail.com>
Sent: Tuesday, May 28, 2024 5:34 PM
To: Feng Xue OS
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] vect: Use vect representative statement instead of original in patch recog [PR115060]

On Sat, May 25, 2024 at 4:45 PM Feng Xue OS <fxue@os.amperecomputing.com> wrote:
>
> Some utility functions (such as vect_look_through_possible_promotion) that are
> to find out certain kind of direct or indirect definition SSA for a value, may
> return the original one of the SSA, not its pattern representative SSA, even
> pattern is involved. For example,
>
>    a = (T1) patt_b;
>    patt_b = (T2) c;        // b = ...
>    patt_c = not-a-cast;    // c = ...
>
> Given 'a', the mentioned function will return 'c', instead of 'patt_c'. This
> subtlety would make some pattern recog code that is unaware of it mis-use the
> original instead of the new pattern statement, which is inconsistent wth
> processing logic of the pattern formation pass. This patch corrects the issue
> by forcing another utility function (vect_get_internal_def) return the pattern
> statement information to caller by default.
>
> Regression test on x86-64 and aarch64.
>
> Feng
> --
> gcc/
>         PR tree-optimization/115060
>         * tree-vect-patterns.h (vect_get_internal_def): Add a new parameter
>         for_vectorize.
>         (vect_widened_op_tree): Call vect_get_internal_def instead of look_def
>         to get statement information.
>         (vect_recog_widen_abd_pattern): No need to call vect_stmt_to_vectorize.
> ---
>  gcc/tree-vect-patterns.cc | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
> index a313dc64643..fa35bf26372 100644
> --- a/gcc/tree-vect-patterns.cc
> +++ b/gcc/tree-vect-patterns.cc
> @@ -258,15 +258,21 @@ vect_element_precision (unsigned int precision)
>  }
>
>  /* If OP is defined by a statement that's being considered for vectorization,
> -   return information about that statement, otherwise return NULL.  */
> +   return information about that statement, otherwise return NULL.
> +   FOR_VECTORIZE is used to specify whether original or vectorization
> +   representative (if have) statement information is returned.  */
>
>  static stmt_vec_info
> -vect_get_internal_def (vec_info *vinfo, tree op)
> +vect_get_internal_def (vec_info *vinfo, tree op, bool for_vectorize = true)

I'm probably blind - but you nowhere pass 'false' and I think returning the
pattern stmt is the correct behavior always.

OK with omitting the new parameter.

>  {
>    stmt_vec_info def_stmt_info = vinfo->lookup_def (op);
>    if (def_stmt_info
>        && STMT_VINFO_DEF_TYPE (def_stmt_info) == vect_internal_def)
> -    return def_stmt_info;
> +    {
> +      if (for_vectorize)
> +       def_stmt_info = vect_stmt_to_vectorize (def_stmt_info);
> +      return def_stmt_info;
> +    }
>    return NULL;
>  }
>
> @@ -655,7 +661,8 @@ vect_widened_op_tree (vec_info *vinfo, stmt_vec_info stmt_info, tree_code code,
>
>               /* Recursively process the definition of the operand.  */
>               stmt_vec_info def_stmt_info
> -               = vinfo->lookup_def (this_unprom->op);
> +               = vect_get_internal_def (vinfo, this_unprom->op);
> +
>               nops = vect_widened_op_tree (vinfo, def_stmt_info, code,
>                                            widened_code, shift_p, max_nops,
>                                            this_unprom, common_type,
> @@ -1739,7 +1746,6 @@ vect_recog_widen_abd_pattern (vec_info *vinfo, stmt_vec_info stmt_vinfo,
>    if (!abd_pattern_vinfo)
>      return NULL;
>
> -  abd_pattern_vinfo = vect_stmt_to_vectorize (abd_pattern_vinfo);
>    gcall *abd_stmt = dyn_cast <gcall *> (STMT_VINFO_STMT (abd_pattern_vinfo));
>    if (!abd_stmt