[PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block

[PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block

[Juzhe-Zhong]

This patch is fixing these bunch of ICE in "vect" testsuite:
FAIL: gcc.dg/vect/no-scevccp-outer-2.c (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/no-scevccp-outer-2.c (test for excess errors)
FAIL: gcc.dg/vect/pr109025.c (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/pr109025.c (test for excess errors)
FAIL: gcc.dg/vect/pr109025.c -flto -ffat-lto-objects (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/pr109025.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/pr42604.c (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/pr42604.c (test for excess errors)
FAIL: gcc.dg/vect/pr42604.c -flto -ffat-lto-objects (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/pr42604.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-double-reduc-3.c (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/vect-double-reduc-3.c (test for excess errors)
FAIL: gcc.dg/vect/vect-double-reduc-3.c -flto -ffat-lto-objects (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/vect-double-reduc-3.c -flto -ffat-lto-objects (test for excess errors)
FAIL: gcc.dg/vect/vect-double-reduc-7.c (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/vect-double-reduc-7.c (test for excess errors)
FAIL: gcc.dg/vect/vect-double-reduc-7.c -flto -ffat-lto-objects (internal compiler error: in anticipatable_occurrence_p, at config/riscv/riscv-vsetvl.cc:314)
FAIL: gcc.dg/vect/vect-double-reduc-7.c -flto -ffat-lto-objects (test for excess errors)


gcc/ChangeLog:

	* config/riscv/riscv-vsetvl.cc (pass_vsetvl::earliest_fusion): Disable user vsetvl fusion into EMPTY block.

---
 gcc/config/riscv/riscv-vsetvl.cc | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 682f795c8e1..984252f8fe3 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3290,6 +3290,24 @@ pass_vsetvl::earliest_fusion (void)
 		{
 		  if (src_block_info.probability
 			== profile_probability::uninitialized ()
+		      /* We don't do fusion across BBs for user explicit
+			 vsetvl instruction for these following reasons:
+
+			  - The user vsetvl instruction is configured as
+			    no side effects that the previous passes
+			    (GSCE, Loop-invariant, ..., etc)
+			    should be able to do a good job on optimization
+			    of user explicit vsetvls so we don't need to
+			    PRE optimization (The user vsetvls should be
+			    on the optimal local already before this pass)
+			    again for user vsetvls in VSETVL PASS here
+			    (Phase 3 && Phase 4).
+
+			  - Allowing user vsetvls be optimized in PRE
+			    optimization here (Phase 3 && Phase 4) will
+			    complicate the codes so much so we prefer user
+			    vsetvls be optimized in post-optimization
+			    (Phase 5 && Phase 6).  */
 		      || vsetvl_insn_p (expr.get_insn ()->rtl ()))
 		    continue;
 		  new_info = expr.global_merge (expr, eg->src->index);
@@ -3317,6 +3335,25 @@ pass_vsetvl::earliest_fusion (void)
 		      prob = profile_probability::uninitialized ();
 		    }
 		  else if (!src_block_info.reaching_out.compatible_p (expr)
+			   /* We don't do fusion across BBs for user explicit
+			      vsetvl instruction for these following reasons:
+
+			       - The user vsetvl instruction is configured as
+				 no side effects that the previous passes
+				 (GSCE, Loop-invariant, ..., etc)
+				 should be able to do a good job on optimization
+				 of user explicit vsetvls so we don't need to
+				 PRE optimization (The user vsetvls should be
+				 on the optimal local already before this pass)
+				 again for user vsetvls in VSETVL PASS here
+				 (Phase 3 && Phase 4).
+
+			       - Allowing user vsetvls be optimized in PRE
+				 optimization here (Phase 3 && Phase 4) will
+				 complicate the codes so much so we prefer user
+				 vsetvls be optimized in post-optimization
+				 (Phase 5 && Phase 6).  */
+			   && !vsetvl_insn_p (expr.get_insn ()->rtl ())
 			   && dest_block_info.probability
 				> src_block_info.probability)
 		    {
-- 
2.36.3

Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block

[Robin Dapp]

>  		      || vsetvl_insn_p (expr.get_insn ()->rtl ()))
>  		    continue;
>  		  new_info = expr.global_merge (expr, eg->src->index);
> @@ -3317,6 +3335,25 @@ pass_vsetvl::earliest_fusion (void)
>  		      prob = profile_probability::uninitialized ();
>  		    }
>  		  else if (!src_block_info.reaching_out.compatible_p (expr)
> +			   /* We don't do fusion across BBs for user explicit
> +			      vsetvl instruction for these following reasons:
> +
> +			       - The user vsetvl instruction is configured as
> +				 no side effects that the previous passes
> +				 (GSCE, Loop-invariant, ..., etc)
> +				 should be able to do a good job on optimization
> +				 of user explicit vsetvls so we don't need to
> +				 PRE optimization (The user vsetvls should be
> +				 on the optimal local already before this pass)
> +				 again for user vsetvls in VSETVL PASS here
> +				 (Phase 3 && Phase 4).
> +
> +			       - Allowing user vsetvls be optimized in PRE
> +				 optimization here (Phase 3 && Phase 4) will
> +				 complicate the codes so much so we prefer user
> +				 vsetvls be optimized in post-optimization
> +				 (Phase 5 && Phase 6).  */
> +			   && !vsetvl_insn_p (expr.get_insn ()->rtl ())
>  			   && dest_block_info.probability
>  				> src_block_info.probability)

This is OK but do we need the same comment twice?  The first one doesn't
seem to refer to a change but also to vsetvl_insn_p (expr.get_insn()...).

And where is the !empty block property enforced?  I would prefer to have
the longer comment at a top level and shortly describe here why
!vsetvl_insn_p ensures we don't have an EMPTY block.  Don't we usually
have a state for this (i.e. empty_p)?

No need for a V2 though, it can also stay as is and be cleaned up later.

Regards
 Robin

Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block

[Kito Cheng]

Is it possible to skip that at the topper level like that?

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 682f795c8e1..654d25de593 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3269,7 +3269,7 @@ pass_vsetvl::earliest_fusion (void)
      for (size_t i = 0; i < m_vector_manager->vector_exprs.length (); i++)
       {
         auto &expr = *m_vector_manager->vector_exprs[i];
-         if (expr.empty_p ())
+         if (expr.empty_p () || vsetvl_insn_p (expr.get_insn ()->rtl ()))
           continue;
         edge eg = INDEX_EDGE (m_vector_manager->vector_edge_list, ed);
         if (eg->src == ENTRY_BLOCK_PTR_FOR_FN (cfun)

Re: Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block

[juzhe.zhong]

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

No. we can't.

My goal is to forbid this following situation:

bb 2:
EMPTY (no any vsetvl and rvv insn)
bb 3:
user vsetvl.

We forbid user vsetvl (bb 3) be move to bb 2 in earliest fusion since user vsetvl has no side effects
and we believe user vsetvl in bb 3 here is early enough.

However, if we skip that as your patch, we will end up with missing this optimization:

bb 2:
user vsetvl a5, a4, e8, mf4...
vle8
vse8

bb 3:
user vsetvl a5, a4, e16, mf2... -----> In phase 1 && phase 2, the local demand fusion will update it into (e32, m1)
vle32
vadd
vse32

If we skip at the top, we will be missing fuse user vsetvl (in bb 3 e32 m1) into user vsetvl (in bb 2 e8 mf4).

Thanks. 


juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-08-28 16:58
To: Robin Dapp
CC: Juzhe-Zhong; gcc-patches; kito.cheng
Subject: Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block
Is it possible to skip that at the topper level like that?
 
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 682f795c8e1..654d25de593 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3269,7 +3269,7 @@ pass_vsetvl::earliest_fusion (void)
      for (size_t i = 0; i < m_vector_manager->vector_exprs.length (); i++)
       {
         auto &expr = *m_vector_manager->vector_exprs[i];
-         if (expr.empty_p ())
+         if (expr.empty_p () || vsetvl_insn_p (expr.get_insn ()->rtl ()))
           continue;
         edge eg = INDEX_EDGE (m_vector_manager->vector_edge_list, ed);
         if (eg->src == ENTRY_BLOCK_PTR_FOR_FN (cfun)
 

Re: Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block

[Kito Cheng]

What about that? I guess I don't really know how to determine if a
block is EMPTY?

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 682f795c8e1..784ab184c72 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3282,6 +3282,10 @@ pass_vsetvl::earliest_fusion (void)
             if (src_block_info.reaching_out.unknown_p ())
               break;

+             if (vsetvl_insn_p (expr.get_insn ()->rtl ())
+                 && src_block_info.reaching_out.empty_p ())
+               continue;
+
             gcc_assert (!(eg->flags & EDGE_ABNORMAL));
             vector_insn_info new_info = vector_insn_info ();
             profile_probability prob = src_block_info.probability;

On Mon, Aug 28, 2023 at 5:09 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> No. we can't.
>
> My goal is to forbid this following situation:
>
> bb 2:
> EMPTY (no any vsetvl and rvv insn)
> bb 3:
> user vsetvl.
>
> We forbid user vsetvl (bb 3) be move to bb 2 in earliest fusion since user vsetvl has no side effects
> and we believe user vsetvl in bb 3 here is early enough.
>
> However, if we skip that as your patch, we will end up with missing this optimization:
>
> bb 2:
> user vsetvl a5, a4, e8, mf4...
> vle8
> vse8
>
> bb 3:
> user vsetvl a5, a4, e16, mf2... -----> In phase 1 && phase 2, the local demand fusion will update it into (e32, m1)
> vle32
> vadd
> vse32
>
> If we skip at the top, we will be missing fuse user vsetvl (in bb 3 e32 m1) into user vsetvl (in bb 2 e8 mf4).
>
> Thanks.
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-08-28 16:58
> To: Robin Dapp
> CC: Juzhe-Zhong; gcc-patches; kito.cheng
> Subject: Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block
> Is it possible to skip that at the topper level like that?
>
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
> index 682f795c8e1..654d25de593 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -3269,7 +3269,7 @@ pass_vsetvl::earliest_fusion (void)
>       for (size_t i = 0; i < m_vector_manager->vector_exprs.length (); i++)
>        {
>          auto &expr = *m_vector_manager->vector_exprs[i];
> -         if (expr.empty_p ())
> +         if (expr.empty_p () || vsetvl_insn_p (expr.get_insn ()->rtl ()))
>            continue;
>          edge eg = INDEX_EDGE (m_vector_manager->vector_edge_list, ed);
>          if (eg->src == ENTRY_BLOCK_PTR_FOR_FN (cfun)
>

Re: Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block

[juzhe.zhong]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

Address comments:
https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628568.html 




juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-08-28 16:58
To: Robin Dapp
CC: Juzhe-Zhong; gcc-patches; kito.cheng
Subject: Re: [PATCH] RISC-V: Disable user vsetvl fusion into EMPTY block
Is it possible to skip that at the topper level like that?
 
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 682f795c8e1..654d25de593 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3269,7 +3269,7 @@ pass_vsetvl::earliest_fusion (void)
      for (size_t i = 0; i < m_vector_manager->vector_exprs.length (); i++)
       {
         auto &expr = *m_vector_manager->vector_exprs[i];
-         if (expr.empty_p ())
+         if (expr.empty_p () || vsetvl_insn_p (expr.get_insn ()->rtl ()))
           continue;
         edge eg = INDEX_EDGE (m_vector_manager->vector_edge_list, ed);
         if (eg->src == ENTRY_BLOCK_PTR_FOR_FN (cfun)