diff mbox series

[v1] Widening-Mul: Take gsi after_labels instead of start_bb for gcall insertion

Message ID 20240611135322.876470-1-pan2.li@intel.com
State New
Headers show
Series [v1] Widening-Mul: Take gsi after_labels instead of start_bb for gcall insertion | expand

Commit Message

Li, Pan2 June 11, 2024, 1:53 p.m. UTC 
From: Pan Li <pan2.li@intel.com>

We inserted the gcall of .SAT_ADD before the gsi_start_bb for avoiding
the ssa def after use ICE issue.  Unfortunately,  there will be the
potential ICE when the first stmt is label.  We cannot insert the gcall
before the label.  Thus,  we take gsi_after_labels to locate the
'really' stmt that the gcall will insert before.

The existing test cases pr115387-1.c and pr115387-2.c cover this change.

The below test suites are passed for this patch.
* The rv64gcv fully regression test with newlib.
* The x86 regression test.
* The x86 bootstrap test.

gcc/ChangeLog:

	* tree-ssa-math-opts.cc (math_opts_dom_walker::after_dom_children):
	Leverage gsi_after_labels instead of gsi_start_bb to skip the
	leading labels of bb.

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/tree-ssa-math-opts.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Biener June 12, 2024, 6:40 a.m. UTC | #1 
On Tue, Jun 11, 2024 at 3:53 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> We inserted the gcall of .SAT_ADD before the gsi_start_bb for avoiding
> the ssa def after use ICE issue.  Unfortunately,  there will be the
> potential ICE when the first stmt is label.  We cannot insert the gcall
> before the label.  Thus,  we take gsi_after_labels to locate the
> 'really' stmt that the gcall will insert before.
>
> The existing test cases pr115387-1.c and pr115387-2.c cover this change.

OK

> The below test suites are passed for this patch.
> * The rv64gcv fully regression test with newlib.
> * The x86 regression test.
> * The x86 bootstrap test.
>
> gcc/ChangeLog:
>
>         * tree-ssa-math-opts.cc (math_opts_dom_walker::after_dom_children):
>         Leverage gsi_after_labels instead of gsi_start_bb to skip the
>         leading labels of bb.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/tree-ssa-math-opts.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc
> index fbb8e0ea306..c09e9006443 100644
> --- a/gcc/tree-ssa-math-opts.cc
> +++ b/gcc/tree-ssa-math-opts.cc
> @@ -6102,7 +6102,7 @@ math_opts_dom_walker::after_dom_children (basic_block bb)
>    for (gphi_iterator psi = gsi_start_phis (bb); !gsi_end_p (psi);
>      gsi_next (&psi))
>      {
> -      gimple_stmt_iterator gsi = gsi_start_bb (bb);
> +      gimple_stmt_iterator gsi = gsi_after_labels (bb);
>        match_unsigned_saturation_add (&gsi, psi.phi ());
>      }
>
> --
> 2.34.1
>
Li, Pan2 June 12, 2024, 6:54 a.m. UTC | #2 
Committed, thanks Richard.

Pan

-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com> 
Sent: Wednesday, June 12, 2024 2:41 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; jeffreyalaw@gmail.com
Subject: Re: [PATCH v1] Widening-Mul: Take gsi after_labels instead of start_bb for gcall insertion

On Tue, Jun 11, 2024 at 3:53 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> We inserted the gcall of .SAT_ADD before the gsi_start_bb for avoiding
> the ssa def after use ICE issue.  Unfortunately,  there will be the
> potential ICE when the first stmt is label.  We cannot insert the gcall
> before the label.  Thus,  we take gsi_after_labels to locate the
> 'really' stmt that the gcall will insert before.
>
> The existing test cases pr115387-1.c and pr115387-2.c cover this change.

OK

> The below test suites are passed for this patch.
> * The rv64gcv fully regression test with newlib.
> * The x86 regression test.
> * The x86 bootstrap test.
>
> gcc/ChangeLog:
>
>         * tree-ssa-math-opts.cc (math_opts_dom_walker::after_dom_children):
>         Leverage gsi_after_labels instead of gsi_start_bb to skip the
>         leading labels of bb.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/tree-ssa-math-opts.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc
> index fbb8e0ea306..c09e9006443 100644
> --- a/gcc/tree-ssa-math-opts.cc
> +++ b/gcc/tree-ssa-math-opts.cc
> @@ -6102,7 +6102,7 @@ math_opts_dom_walker::after_dom_children (basic_block bb)
>    for (gphi_iterator psi = gsi_start_phis (bb); !gsi_end_p (psi);
>      gsi_next (&psi))
>      {
> -      gimple_stmt_iterator gsi = gsi_start_bb (bb);
> +      gimple_stmt_iterator gsi = gsi_after_labels (bb);
>        match_unsigned_saturation_add (&gsi, psi.phi ());
>      }
>
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc
index fbb8e0ea306..c09e9006443 100644
--- a/gcc/tree-ssa-math-opts.cc
+++ b/gcc/tree-ssa-math-opts.cc
@@ -6102,7 +6102,7 @@  math_opts_dom_walker::after_dom_children (basic_block bb)
   for (gphi_iterator psi = gsi_start_phis (bb); !gsi_end_p (psi);
     gsi_next (&psi))
     {
-      gimple_stmt_iterator gsi = gsi_start_bb (bb);
+      gimple_stmt_iterator gsi = gsi_after_labels (bb);
       match_unsigned_saturation_add (&gsi, psi.phi ());
     }