RISC-V: Forbidden fuse vlmax vsetvl to DEMAND_NONZERO_AVL vsetvl

Message ID 20230817075655.165144-1-lehua.ding@rivai.ai
State Unresolved
Headers
Series RISC-V: Forbidden fuse vlmax vsetvl to DEMAND_NONZERO_AVL vsetvl |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Lehua Ding Aug. 17, 2023, 7:56 a.m. UTC
  Hi,

This little patch fix the fail testcase
(gcc.target/riscv/rvv/autovec/gather-scatter/strided_load_run-1.c)
after apply this patch
(https://gcc.gnu.org/pipermail/gcc-patches/2023-August/627121.html).
The specific reason is that the vsetvl pass has bug and this patch
forbidden the fuse of this case. This patch needs to be committed
before that patch to work.

Best,
Lehua

gcc/ChangeLog:

	* config/riscv/riscv-vsetvl.cc (pass_vsetvl::backward_demand_fusion):
	  Forbidden.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/autovec/gather-scatter/strided_load_run-1.c:
	  Address failure due to uninitialized vtype register. 

---
 gcc/config/riscv/riscv-vsetvl.cc                               | 3 +++
 .../riscv/rvv/autovec/gather-scatter/strided_load_run-1.c      | 1 +
 2 files changed, 4 insertions(+)
  

Comments

Robin Dapp Aug. 17, 2023, 10:05 a.m. UTC | #1
Hi Lehua,

unrelated but I'm seeing a lot of failing gather/scatter tests on
master right now.

>  	      /* DIRTY -> DIRTY or VALID -> DIRTY.  */
> +	      if (block_info.reaching_out.demand_p (DEMAND_NONZERO_AVL)
> +		  && vlmax_avl_p (prop.get_avl ()))
> +		continue;
>  	      vector_insn_info new_info; 

Please add a small comment here which exact situation we're trying
to prevent.

> +asm volatile ("vsetivli x0, 0, e8, m1, ta, ma");

Why is this necessary or rather why is vtype uninitialized?  Is
this the mentioned bug?  If so, why do we still need it with the
vsetvl fix? 

Regards
 Robin
  
Lehua Ding Aug. 17, 2023, 10:54 a.m. UTC | #2
Hi Robin,


> unrelated but I'm seeing a lot of failing gather/scatter tests on
> master right now.


Are you talking about these FAILs like bellow? If so, If so it should be caused by a
recent commit from juzhe who is looking at it. If not, I didn't have these fails
in my local run.


  XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
  XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
  XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
  XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand



> Please add a small comment here which exact situation we're trying
> to prevent.


OK, will add a comment like bellow:


	      /* Forbidden this case be fused because it change the value of a5.
		   bb 1: vsetvl zero, no_zero_avl
			 ...
			 use a5
			 ...
		   bb 2: vsetvl a5, zero
		 =>
		   bb 1: vsetvl a5, zero
			 ...
			 use a5
			 ...
		   bb 2:
	      */





> Why is this necessary or rather why is vtype uninitialized?  Is
> this the mentioned bug?  If so, why do we still need it with the
> vsetvl fix?


This is because running a testcase with spike+pk will result in an
ILLEGAL INSTRUCTION error if the vtype registers are not initialized
before executing vmv1r.v instruction. This case fails because of this reason,
so explicitly execute vsetvl early. We are currently discussing with Kito to
constrain this case in psABI and ask the execution environment(pk) to ensure
that vtype is initialized, but not so fast. So when encountering a testcase that
fails because of this reason, I think use this way to fix it is ok.


------------------ Original ------------------
From:                                                                                                                        "Robin Dapp"                                                                                    <gcc-patches@gcc.gnu.org&gt;;
Date:&nbsp;Thu, Aug 17, 2023 06:05 PM
To:&nbsp;"Lehua Ding"<lehua.ding@rivai.ai&gt;;"gcc-patches"<gcc-patches@gcc.gnu.org&gt;;
Cc:&nbsp;"rdapp.gcc"<rdapp.gcc@gmail.com&gt;;"juzhe.zhong"<juzhe.zhong@rivai.ai&gt;;"kito.cheng"<kito.cheng@gmail.com&gt;;"palmer"<palmer@rivosinc.com&gt;;"jeffreyalaw"<jeffreyalaw@gmail.com&gt;;
Subject:&nbsp;Re: [PATCH] RISC-V: Forbidden fuse vlmax vsetvl to DEMAND_NONZERO_AVL vsetvl



Hi Lehua,

unrelated but I'm seeing a lot of failing gather/scatter tests on
master right now.

&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /* DIRTY -&gt; DIRTY or VALID -&gt; DIRTY.&nbsp; */
&gt; + &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (block_info.reaching_out.demand_p (DEMAND_NONZERO_AVL)
&gt; +&nbsp; &amp;&amp; vlmax_avl_p (prop.get_avl ()))
&gt; +continue;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; vector_insn_info new_info; 

Please add a small comment here which exact situation we're trying
to prevent.

&gt; +asm volatile ("vsetivli x0, 0, e8, m1, ta, ma");

Why is this necessary or rather why is vtype uninitialized?&nbsp; Is
this the mentioned bug?&nbsp; If so, why do we still need it with the
vsetvl fix? 

Regards
Robin
  
Lehua Ding Aug. 17, 2023, 11:04 a.m. UTC | #3
I see these failing testcases on trunk:


&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; === gcc: Unexpected fails for rv64gcv_zfh lp64d medany spike ===
FAIL: gcc.dg/pr42685.c (test for excess errors)
FAIL: gcc.dg/pr45105.c (test for excess errors)
XPASS: gcc.dg/unroll-7.c scan-rtl-dump-not loop2_unroll "Invalid sum"
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 17)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 18)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 21)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 31)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 32)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 35)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 45)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 55)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 63)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 66)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 68)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so&nbsp; (test for warnings, line 69)
FAIL: gcc.dg/plugin/cpython-plugin-test-2.c -fplugin=./analyzer_cpython_plugin.so (test for excess errors)
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-16.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-tree-dump-times optimized ".VEC_PERM" 2
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-tree-dump-times optimized ".VEC_PERM" 2
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-tree-dump-times optimized ".VEC_PERM" 2
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-tree-dump-times optimized ".VEC_PERM" 2
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-tree-dump-times optimized ".VEC_PERM" 2
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-17.c scan-tree-dump-times optimized ".VEC_PERM" 2
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-18.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-18.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-18.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-18.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-assembler \\tvid\\.v
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-tree-dump optimized ".VEC_PERM"
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-tree-dump optimized ".VEC_PERM"
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-tree-dump optimized ".VEC_PERM"
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-19.c scan-tree-dump optimized ".VEC_PERM"
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-2.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-2.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-2.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-2.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-2.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-2.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-3.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-3.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-3.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-3.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-4.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-4.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-4.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-4.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-4.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-4.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-5.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-5.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-5.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-5.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-6.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-6.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-6.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-6.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-6.c scan-tree-dump-times optimized ".VEC_PERM" 1
XPASS: gcc.target/riscv/rvv/autovec/partial/slp-6.c scan-tree-dump-times optimized ".VEC_PERM" 1
FAIL: gcc.target/riscv/rvv/autovec/vls/const-4.c -O3 -ftree-vectorize --param riscv-autovec-preference=scalable&nbsp; scan-assembler-times vfmv\\.v\\.f\\s+v[0-9]+,\\s*[a-x0-9]+ 30







------------------&nbsp;Original&nbsp;------------------
From:                                                                                                                        "Lehua Ding"                                                                                    <lehua.ding@rivai.ai&gt;;
Date:&nbsp;Thu, Aug 17, 2023 06:54 PM
To:&nbsp;"Robin Dapp"<rdapp.gcc@gmail.com&gt;;"gcc-patches"<gcc-patches@gcc.gnu.org&gt;;
Cc:&nbsp;"rdapp.gcc"<rdapp.gcc@gmail.com&gt;;"juzhe.zhong"<juzhe.zhong@rivai.ai&gt;;"kito.cheng"<kito.cheng@gmail.com&gt;;"palmer"<palmer@rivosinc.com&gt;;"jeffreyalaw"<jeffreyalaw@gmail.com&gt;;
Subject:&nbsp;Re: [PATCH] RISC-V: Forbidden fuse vlmax vsetvl to DEMAND_NONZERO_AVL vsetvl



Hi Robin,


&amp;gt; unrelated but I'm seeing a lot of failing gather/scatter tests on
&amp;gt; master right now.


Are you talking about these FAILs like bellow? If so,&amp;nbsp;If so it should be caused by a
recent commit from juzhe who is looking at it.&amp;nbsp;If not,&amp;nbsp;I didn't have these fails
in my local run.


 XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
 XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
 XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
 XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand



&amp;gt; Please add a small comment here which exact situation we're trying
&amp;gt; to prevent.


OK, will add a comment like bellow:


 &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /* Forbidden this case be fused because it change the value of a5.
&nbsp;&nbsp; bb 1: vsetvl zero, no_zero_avl
...
use a5
...
&nbsp;&nbsp; bb 2: vsetvl a5, zero
 =&amp;gt;
&nbsp;&nbsp; bb 1: vsetvl a5, zero
...
use a5
...
&nbsp;&nbsp; bb 2:
 &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; */





&amp;gt; Why is this necessary or rather why is vtype uninitialized?&amp;nbsp; Is
&amp;gt; this the mentioned bug?&amp;nbsp; If so, why do we still need it with the
&amp;gt; vsetvl fix?


This is because running a testcase with spike+pk will result in an
ILLEGAL INSTRUCTION error if the vtype registers are not initialized
before executing vmv1r.v instruction.&amp;nbsp;This case fails because of this reason,
so explicitly execute vsetvl early. We are currently discussing with Kito to
constrain this case in psABI and ask the execution environment(pk) to ensure
that vtype is initialized, but not so fast. So when encountering a testcase that
fails because of this reason, I think use this way to fix it is ok.


------------------&amp;nbsp;Original&amp;nbsp;------------------
From:&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; "Robin Dapp"&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; <gcc-patches@gcc.gnu.org&amp;gt;;
Date:&amp;nbsp;Thu, Aug 17, 2023 06:05 PM
To:&amp;nbsp;"Lehua Ding"<lehua.ding@rivai.ai&amp;gt;;"gcc-patches"<gcc-patches@gcc.gnu.org&amp;gt;;
Cc:&amp;nbsp;"rdapp.gcc"<rdapp.gcc@gmail.com&amp;gt;;"juzhe.zhong"<juzhe.zhong@rivai.ai&amp;gt;;"kito.cheng"<kito.cheng@gmail.com&amp;gt;;"palmer"<palmer@rivosinc.com&amp;gt;;"jeffreyalaw"<jeffreyalaw@gmail.com&amp;gt;;
Subject:&amp;nbsp;Re: [PATCH] RISC-V: Forbidden fuse vlmax vsetvl to DEMAND_NONZERO_AVL vsetvl



Hi Lehua,

unrelated but I'm seeing a lot of failing gather/scatter tests on
master right now.

&amp;gt;&amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp; /* DIRTY -&amp;gt; DIRTY or VALID -&amp;gt; DIRTY.&amp;nbsp; */
&amp;gt; + &amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp; if (block_info.reaching_out.demand_p (DEMAND_NONZERO_AVL)
&amp;gt; +&amp;nbsp; &amp;amp;&amp;amp; vlmax_avl_p (prop.get_avl ()))
&amp;gt; +continue;
&amp;gt;&amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp;&amp;nbsp; vector_insn_info new_info; 

Please add a small comment here which exact situation we're trying
to prevent.

&amp;gt; +asm volatile ("vsetivli x0, 0, e8, m1, ta, ma");

Why is this necessary or rather why is vtype uninitialized?&amp;nbsp; Is
this the mentioned bug?&amp;nbsp; If so, why do we still need it with the
vsetvl fix? 

Regards
Robin
  
Robin Dapp Aug. 17, 2023, 12:15 p.m. UTC | #4
Hi Lehua,

> XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
> XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
> XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
> XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand

Thanks for checking, I know about those but have other FAILs.  Probably
due to a recent update or so, need to check.

> This is because running a testcase with spike+pk will result in an
> ILLEGAL INSTRUCTION error if the vtype registers are not initialized
> before executing vmv1r.v instruction. This case fails because of this reason,
> so explicitly execute vsetvl early. We are currently discussing with Kito to
> constrain this case in psABI and ask the execution environment(pk) to ensure
> that vtype is initialized, but not so fast. So when encountering a testcase that
> fails because of this reason, I think use this way to fix it is ok.

Hmm, ok so that has nothing to do with the rest of the patch but just
happend to be the same test case.
So we didn't schedule a vsetvl here because vmv1r doesn't require
one but the simulation doesn't initialize vtype before the first vsetvl?
If this is the only instance, I guess that's OK, but please add a comment
as well.

OK with the two comments added.

Regards
 Robin
  
Lehua Ding Aug. 17, 2023, 12:21 p.m. UTC | #5
Hi Robin,


&gt; Hmm, ok so that has nothing to do with the rest of the patch but just
&gt; happend to be the same test case.
&gt; So we didn't schedule a vsetvl here because vmv1r doesn't require
&gt; one but the simulation doesn't initialize vtype before the first vsetvl?
&gt; If this is the only instance, I guess that's OK, but please add a comment
&gt; as well.


Understood exactly right. There should be a harmonized solution to
this problem later. This is an interim solution for reduce unnecessary failures..


Best,
Lehua


------------------&nbsp;Original&nbsp;------------------
From:                                                                                                                        "Robin Dapp"                                                                                    <gcc-patches@gcc.gnu.org&gt;;
Date:&nbsp;Thu, Aug 17, 2023 08:15 PM
To:&nbsp;"Lehua Ding"<lehua.ding@rivai.ai&gt;;"gcc-patches"<gcc-patches@gcc.gnu.org&gt;;
Cc:&nbsp;"rdapp.gcc"<rdapp.gcc@gmail.com&gt;;"juzhe.zhong"<juzhe.zhong@rivai.ai&gt;;"kito.cheng"<kito.cheng@gmail.com&gt;;"palmer"<palmer@rivosinc.com&gt;;"jeffreyalaw"<jeffreyalaw@gmail.com&gt;;
Subject:&nbsp;Re: [PATCH] RISC-V: Forbidden fuse vlmax vsetvl to DEMAND_NONZERO_AVL vsetvl



Hi Lehua,

&gt; XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
&gt; XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
&gt; XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand
&gt; XPASS: gcc.target/riscv/rvv/autovec/partial/slp-1.c scan-assembler \\tvand

Thanks for checking, I know about those but have other FAILs.&nbsp; Probably
due to a recent update or so, need to check.

&gt; This is because running a testcase with spike+pk will result in an
&gt; ILLEGAL INSTRUCTION error if the vtype registers are not initialized
&gt; before executing vmv1r.v instruction.&nbsp;This case fails because of this reason,
&gt; so explicitly execute vsetvl early. We are currently discussing with Kito to
&gt; constrain this case in psABI and ask the execution environment(pk) to ensure
&gt; that vtype is initialized, but not so fast. So when encountering a testcase that
&gt; fails because of this reason, I think use this way to fix it is ok.

Hmm, ok so that has nothing to do with the rest of the patch but just
happend to be the same test case.
So we didn't schedule a vsetvl here because vmv1r doesn't require
one but the simulation doesn't initialize vtype before the first vsetvl?
If this is the only instance, I guess that's OK, but please add a comment
as well.

OK with the two comments added.

Regards
Robin
  

Patch

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 08c487d82c0..11ef5d628c4 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3312,6 +3312,9 @@  pass_vsetvl::backward_demand_fusion (void)
 	  else if (block_info.reaching_out.dirty_p ())
 	    {
 	      /* DIRTY -> DIRTY or VALID -> DIRTY.  */
+	      if (block_info.reaching_out.demand_p (DEMAND_NONZERO_AVL)
+		  && vlmax_avl_p (prop.get_avl ()))
+		continue;
 	      vector_insn_info new_info;
 
 	      if (block_info.reaching_out.compatible_p (prop))
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/strided_load_run-1.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/strided_load_run-1.c
index 7ffa93bf13f..5080f196601 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/strided_load_run-1.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/strided_load_run-1.c
@@ -7,6 +7,7 @@ 
 int
 main (void)
 {
+asm volatile ("vsetivli x0, 0, e8, m1, ta, ma");
 #define RUN_LOOP(DATA_TYPE, BITS)                                              \
   DATA_TYPE dest_##DATA_TYPE##_##BITS[(BITS - 3) * (BITS + 13)];               \
   DATA_TYPE dest2_##DATA_TYPE##_##BITS[(BITS - 3) * (BITS + 13)];              \