[v1] RISC-V: Align the predictor style for define_insn_and_split

Message ID 20230614021557.1691461-1-pan2.li@intel.com
State Unresolved
Headers
Series [v1] RISC-V: Align the predictor style for define_insn_and_split |

Checks

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

Commit Message

Li, Pan2 via Gcc-patches June 14, 2023, 2:15 a.m. UTC
  From: Pan Li <pan2.li@intel.com>

This patch is considered as the follow up of the below PATCH.

https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621347.html

We aligned the predictor style for the define_insn_and_split suggested
by Kito. To avoid potential issues before we hit.

Signed-off-by: Pan Li <pan2.li@intel.com>

gcc/ChangeLog:

	* config/riscv/autovec-opt.md: Align the predictor sytle.
	* config/riscv/autovec.md: Ditto.
---
 gcc/config/riscv/autovec-opt.md | 20 ++++++++++----------
 gcc/config/riscv/autovec.md     | 24 ++++++++++++------------
 2 files changed, 22 insertions(+), 22 deletions(-)
  

Comments

juzhe.zhong@rivai.ai June 14, 2023, 2:31 a.m. UTC | #1
LGTM. 



juzhe.zhong@rivai.ai
 
From: pan2.li
Date: 2023-06-14 10:15
To: gcc-patches
CC: juzhe.zhong; rdapp.gcc; jeffreyalaw; pan2.li; yanzhang.wang; kito.cheng
Subject: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split
From: Pan Li <pan2.li@intel.com>
 
This patch is considered as the follow up of the below PATCH.
 
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621347.html
 
We aligned the predictor style for the define_insn_and_split suggested
by Kito. To avoid potential issues before we hit.
 
Signed-off-by: Pan Li <pan2.li@intel.com>
 
gcc/ChangeLog:
 
* config/riscv/autovec-opt.md: Align the predictor sytle.
* config/riscv/autovec.md: Ditto.
---
gcc/config/riscv/autovec-opt.md | 20 ++++++++++----------
gcc/config/riscv/autovec.md     | 24 ++++++++++++------------
2 files changed, 22 insertions(+), 22 deletions(-)
 
diff --git a/gcc/config/riscv/autovec-opt.md b/gcc/config/riscv/autovec-opt.md
index aef28e445e1..fb1b07205aa 100644
--- a/gcc/config/riscv/autovec-opt.md
+++ b/gcc/config/riscv/autovec-opt.md
@@ -37,9 +37,9 @@ (define_insn_and_split "@pred_single_widen_mul<any_extend:su><mode>"
      (match_operand:<V_DOUBLE_TRUNC> 4 "register_operand" "   vr,   vr"))
    (match_operand:VWEXTI 3 "register_operand"             "   vr,   vr"))
  (match_operand:VWEXTI 2 "vector_merge_operand"           "   vu,    0")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_vf2 (<CODE>, <MODE>mode);
@@ -132,9 +132,9 @@ (define_insn_and_split "*<optab>not<mode>"
(bitmanip_bitwise:VB
  (not:VB (match_operand:VB 2 "register_operand" " vr"))
  (match_operand:VB 1 "register_operand"         " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_not (<CODE>, <MODE>mode);
@@ -159,9 +159,9 @@ (define_insn_and_split "*n<optab><mode>"
  (any_bitwise:VB
    (match_operand:VB 1 "register_operand" " vr")
    (match_operand:VB 2 "register_operand" " vr"))))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_n (<CODE>, <MODE>mode);
@@ -346,9 +346,9 @@ (define_insn_and_split "*v<any_shiftrt:optab><any_extend:optab>trunc<mode>"
         (match_operand:VWEXTI 1 "register_operand"                 " vr,vr")
(any_extend:VWEXTI
           (match_operand:<V_DOUBLE_TRUNC> 2 "vector_shift_operand" " vr,vk")))))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
{
   insn_code icode = code_for_pred_narrow (<any_shiftrt:CODE>, <MODE>mode);
@@ -364,9 +364,9 @@ (define_insn_and_split "*<any_shiftrt:optab>trunc<mode>"
       (any_shiftrt:VWEXTI
         (match_operand:VWEXTI 1 "register_operand"           " vr")
(match_operand:<VEL> 2 "csr_operand"                 " rK"))))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
{
   operands[2] = gen_lowpart (Pmode, operands[2]);
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index eadc2c5b595..c23a625afe1 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -155,9 +155,9 @@ (define_insn_and_split "<optab><mode>3"
     (any_shift:VI
      (match_operand:VI 1 "register_operand"    " vr")
      (match_operand:<VEL> 2 "csr_operand"      " rK")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
{
   operands[2] = gen_lowpart (Pmode, operands[2]);
@@ -180,9 +180,9 @@ (define_insn_and_split "v<optab><mode>3"
     (any_shift:VI
      (match_operand:VI 1 "register_operand"     " vr,vr")
      (match_operand:VI 2 "vector_shift_operand" " vr,vk")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
{
   riscv_vector::emit_vlmax_insn (code_for_pred (<CODE>, <MODE>mode),
@@ -205,9 +205,9 @@ (define_insn_and_split "<optab><mode>3"
   [(set (match_operand:VB 0 "register_operand"                 "=vr")
(any_bitwise:VB (match_operand:VB 1 "register_operand" " vr")
(match_operand:VB 2 "register_operand" " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred (<CODE>, <MODE>mode);
@@ -227,9 +227,9 @@ (define_insn_and_split "<optab><mode>3"
(define_insn_and_split "one_cmpl<mode>2"
   [(set (match_operand:VB 0 "register_operand"         "=vr")
(not:VB (match_operand:VB 1 "register_operand" " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_not (<MODE>mode);
@@ -366,9 +366,9 @@ (define_insn_and_split "<optab><v_double_trunc><mode>2"
   [(set (match_operand:VWEXTI 0 "register_operand" "=&vr")
     (any_extend:VWEXTI
      (match_operand:<V_DOUBLE_TRUNC> 1 "register_operand" "vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
{
   insn_code icode = code_for_pred_vf2 (<CODE>, <MODE>mode);
@@ -409,9 +409,9 @@ (define_insn_and_split "trunc<mode><v_double_trunc>2"
   [(set (match_operand:<V_DOUBLE_TRUNC> 0 "register_operand" "=vr")
     (truncate:<V_DOUBLE_TRUNC>
      (match_operand:VWEXTI 1 "register_operand"              " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
{
   insn_code icode = code_for_pred_trunc (<MODE>mode);
-- 
2.34.1
  
Li, Pan2 via Gcc-patches June 14, 2023, 6 a.m. UTC | #2
Thanks Juzhe, Just passed the RV64 riscv/rvv.exp but meet some failures in RV32 the same as upstream. However, this patch may not introduce new failures but I am not quite sure if there is risk here.

lowlist `find build-gcc-newlib-stage2/gcc/testsuite/ -name *.sum |paste -sd "," -`
                === gcc: Unexpected fails for rv32imafdcv ilp32f medlow ===
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/repeat_run-3.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax execution test
FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-run.c -O3 -ftree-vectorize (test for excess errors)
                === g++: Unexpected fails for rv32imafdcv ilp32f medlow ===
FAIL: g++.target/riscv/rvv/base/bug-14.C (test for excess errors)
FAIL: g++.target/riscv/rvv/base/bug-9.C (test for excess errors)

               ========= Summary of gcc testsuite =========
                            | # of unexpected case / # of unique unexpected case
                            |          gcc |          g++ |     gfortran |
rv32imafdcv/ ilp32f/ medlow |    7 /     4 |    2 /     2 |      - |

For RV32, mostly I take below commands for testing.

cd riscv-gnu-toolchain
cd gcc && git checkout master && git pull -p && cd -
cd spike && git checkout master && git pull -p && cd -
cd pk && git checkout master && git pull -p && cd -

./configure --prefix=`pwd`/__RISC-V_INSTALL_/ --with-arch=rv32imafdcv --with-abi=ilp32f --with-isa-spec=20191213 --with-sim=spike
make -j $(nproc) build-sim SIM=spike
make report -j $(nproc) RUNTESTFLAGS="rvv.exp"

Pan

From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai>
Sent: Wednesday, June 14, 2023 10:31 AM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Robin Dapp <rdapp.gcc@gmail.com>; jeffreyalaw <jeffreyalaw@gmail.com>; Li, Pan2 <pan2.li@intel.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split

LGTM.
  
Robin Dapp June 14, 2023, 6:09 a.m. UTC | #3
Hi Pan,

these failures were present before the patch I suppose? They
don't look related.  Is this what you meant by "the same as upstream"?

> FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax (test for excess errors)

This one should probably be 64-bit only.  Would you mind adjusting it?

> FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-run.c -O3 -ftree-vectorize (test for excess errors)

What's happening here?  Any details on the output?

I don't have a proper sim environment setup yet.  How long does the testsuite take
with spike?  Have you tried qemu as well?

Regards
 Robin
  
juzhe.zhong@rivai.ai June 14, 2023, 6:30 a.m. UTC | #4
All failures with (test for excess errors) are not big issues which are created by testcase themselves.

For example:
FAIL: g++.target/riscv/rvv/base/bug-14.C (test for excess errors)
FAIL: g++.target/riscv/rvv/base/bug-9.C (test for excess errors)
These 2 failures are because RV32 doesn't have indexed load/store with indexed EEW = 64.
Like __riscv_vsuxei64_v_i32mf2    in bug-14.C, this intrinsic is valid in RV64 but invalid in RV32 which
is totally correct since according to RVV ISA:
The V extension supports all vector load and store instructions (Section Vector Loads and Stores), except the V extension does not support EEW=64 for index values when XLEN=32.

So these 2 failures in RV32 are not the compile's bugs. I have seen:  /* { dg-do run { target { { {riscv_vector} && {rv64} } } } } */
in these testcases which can not work to block execution in RV32 (Since such testcase only needs to be tested on RV64). I think this is the issue we need to figure out.

So, to conclude:
All these failures:
                === gcc: Unexpected fails for rv32imafdcv ilp32f medlow ===
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-run.c -O3 -ftree-vectorize (test for excess errors)
                === g++: Unexpected fails for rv32imafdcv ilp32f medlow ===
FAIL: g++.target/riscv/rvv/base/bug-14.C (test for excess errors)
FAIL: g++.target/riscv/rvv/base/bug-9.C (test for excess errors)

These failures are not compiler's bugs, should be testcase or test framework issues.

The only issues related to compiler are these:
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/repeat_run-3.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax execution test

These 2 issues I already noticed which should be already fixed by your another patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621610.html 

Overal, this patch doesn't cause any issues.  So, we can go ahead.


Thanks.


juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-06-14 14:09
To: Li, Pan2; juzhe.zhong@rivai.ai; gcc-patches
CC: rdapp.gcc; jeffreyalaw; Wang, Yanzhang; kito.cheng
Subject: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split
Hi Pan,
 
these failures were present before the patch I suppose? They
don't look related.  Is this what you meant by "the same as upstream"?
 
> FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax (test for excess errors)
 
This one should probably be 64-bit only.  Would you mind adjusting it?
 
> FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-run.c -O3 -ftree-vectorize (test for excess errors)
 
What's happening here?  Any details on the output?
 
I don't have a proper sim environment setup yet.  How long does the testsuite take
with spike?  Have you tried qemu as well?
 
Regards
Robin
  
Li, Pan2 via Gcc-patches June 14, 2023, 6:36 a.m. UTC | #5
Thanks Juzhe for explanation, that make more sense to me and sorry for disturbing.

Pan

From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai>
Sent: Wednesday, June 14, 2023 2:31 PM
To: Robin Dapp <rdapp.gcc@gmail.com>; Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Robin Dapp <rdapp.gcc@gmail.com>; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split

All failures with (test for excess errors) are not big issues which are created by testcase themselves.

For example:
FAIL: g++.target/riscv/rvv/base/bug-14.C (test for excess errors)
FAIL: g++.target/riscv/rvv/base/bug-9.C (test for excess errors)
These 2 failures are because RV32 doesn't have indexed load/store with indexed EEW = 64.
Like __riscv_vsuxei64_v_i32mf2    in bug-14.C, this intrinsic is valid in RV64 but invalid in RV32 which
is totally correct since according to RVV ISA:
The V extension supports all vector load and store instructions (Section Vector Loads and Stores<https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#sec-vector-memory>), except the V extension does not support EEW=64 for index values when XLEN=32.

So these 2 failures in RV32 are not the compile's bugs. I have seen:  /* { dg-do run { target { { {riscv_vector} && {rv64} } } } } */
in these testcases which can not work to block execution in RV32 (Since such testcase only needs to be tested on RV64). I think this is the issue we need to figure out.

So, to conclude:
All these failures:
                === gcc: Unexpected fails for rv32imafdcv ilp32f medlow ===
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax (test for excess errors)
FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-run.c -O3 -ftree-vectorize (test for excess errors)
                === g++: Unexpected fails for rv32imafdcv ilp32f medlow ===
FAIL: g++.target/riscv/rvv/base/bug-14.C (test for excess errors)
FAIL: g++.target/riscv/rvv/base/bug-9.C (test for excess errors)

These failures are not compiler's bugs, should be testcase or test framework issues.

The only issues related to compiler are these:
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/repeat_run-3.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax execution test

These 2 issues I already noticed which should be already fixed by your another patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621610.html


Overal, this patch doesn't cause any issues.  So, we can go ahead.


Thanks.
________________________________
juzhe.zhong@rivai.ai<mailto:juzhe.zhong@rivai.ai>

From: Robin Dapp<mailto:rdapp.gcc@gmail.com>
Date: 2023-06-14 14:09
To: Li, Pan2<mailto:pan2.li@intel.com>; juzhe.zhong@rivai.ai<mailto:juzhe.zhong@rivai.ai>; gcc-patches<mailto:gcc-patches@gcc.gnu.org>
CC: rdapp.gcc<mailto:rdapp.gcc@gmail.com>; jeffreyalaw<mailto:jeffreyalaw@gmail.com>; Wang, Yanzhang<mailto:yanzhang.wang@intel.com>; kito.cheng<mailto:kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split
Hi Pan,

these failures were present before the patch I suppose? They
don't look related.  Is this what you meant by "the same as upstream"?

> FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/full-vec-move1.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax (test for excess errors)

This one should probably be 64-bit only.  Would you mind adjusting it?

> FAIL: gcc.target/riscv/rvv/autovec/vmv-imm-run.c -O3 -ftree-vectorize (test for excess errors)

What's happening here?  Any details on the output?

I don't have a proper sim environment setup yet.  How long does the testsuite take
with spike?  Have you tried qemu as well?

Regards
Robin
  
Robin Dapp June 14, 2023, 6:47 a.m. UTC | #6
>     I don't have a proper sim environment setup yet.  How long does the testsuite take
>     with spike?  Have you tried qemu as well?
Any numbers on this Pan? How many cores do you use for running the testsuite?

Regards
 Robin
  
Li, Pan2 via Gcc-patches June 14, 2023, 6:51 a.m. UTC | #7
> Any numbers on this Pan? How many cores do you use for running the testsuite?
Sorry for missing this part. It takes about 4-6 minutes with spike and 16 cores.

Pan

-----Original Message-----
From: Robin Dapp <rdapp.gcc@gmail.com> 
Sent: Wednesday, June 14, 2023 2:47 PM
To: Li, Pan2 <pan2.li@intel.com>; juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: rdapp.gcc@gmail.com; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split

>     I don't have a proper sim environment setup yet.  How long does the testsuite take
>     with spike?  Have you tried qemu as well?
Any numbers on this Pan? How many cores do you use for running the testsuite?

Regards
 Robin
  
Robin Dapp June 14, 2023, 6:52 a.m. UTC | #8
Yes, I agree with the general assessment (and didn't mean to insinuate
that the FAILs are compiler's or a fault of the patch.

> So these 2 failures in RV32 are not the compile's bugs. I have seen:
> /* { dg-do run { target { { {riscv_vector} && {rv64} } } } } */ in
> these testcases which can not work to block execution in RV32 (Since
> such testcase only needs to be tested on RV64). I think this is the
> issue we need to figure out.

Yeah sure, we need to be able to run tests only for specific targets.
Why does {riscv_vector} && {rv64} not work?

For zvfh I'm testing something like the following:

 proc check_effective_target_riscv_zvfh { } {
    if { ![istarget rv32*-*-*] && ![istarget rv64*-*-*] } then {
	return 0;
    }

    if !check_effective_target_riscv_vector then {
	return 0;
    }

    return [
	[check_runtime riscv_check_zvfh {
	    int main (void)
	    {
		asm ("vsetivli zero,8,e16,m1,ta,ma");
		asm ("vfadd.vv %%v8,%%v8,%%v16" : : : "%%v8");
		return 0;
	    }
	} "-march=rv64gcv_zvfh" ]
	|| ... ]

Regards
 Robin
  
juzhe.zhong@rivai.ai June 14, 2023, 7:01 a.m. UTC | #9
>> Yeah sure, we need to be able to run tests only for specific targets.
>> Why does {riscv_vector} && {rv64} not work?
I am not sure. These testcases were added by kito long time ago.
Frankly, I am not familiar with GCC test framework.

I think the highest priority is to fix the "real" compiler bugs which I have noticed yesterday:
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/repeat_run-3.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax execution test

@Li Pan could you verify whether your patch
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621610.html can fix these 2 issues?
If yes, please send V2 patch with append these information into patch log.


Thanks.


juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-06-14 14:52
To: juzhe.zhong@rivai.ai; pan2.li; gcc-patches
CC: rdapp.gcc; jeffreyalaw; yanzhang.wang; kito.cheng
Subject: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split
Yes, I agree with the general assessment (and didn't mean to insinuate
that the FAILs are compiler's or a fault of the patch.
 
> So these 2 failures in RV32 are not the compile's bugs. I have seen:
> /* { dg-do run { target { { {riscv_vector} && {rv64} } } } } */ in
> these testcases which can not work to block execution in RV32 (Since
> such testcase only needs to be tested on RV64). I think this is the
> issue we need to figure out.
 
Yeah sure, we need to be able to run tests only for specific targets.
Why does {riscv_vector} && {rv64} not work?
 
For zvfh I'm testing something like the following:
 
proc check_effective_target_riscv_zvfh { } {
    if { ![istarget rv32*-*-*] && ![istarget rv64*-*-*] } then {
return 0;
    }
 
    if !check_effective_target_riscv_vector then {
return 0;
    }
 
    return [
[check_runtime riscv_check_zvfh {
    int main (void)
    {
asm ("vsetivli zero,8,e16,m1,ta,ma");
asm ("vfadd.vv %%v8,%%v8,%%v16" : : : "%%v8");
return 0;
    }
} "-march=rv64gcv_zvfh" ]
|| ... ]
 
Regards
Robin
  
Robin Dapp June 14, 2023, 7:02 a.m. UTC | #10
> I am not sure. These testcases were added by kito long time ago.
> Frankly, I am not familiar with GCC test framework.

Ok, I'm going to have a look.  Need to verify the zvfh things anyway.

Regards
 Robin
  
Li, Pan2 via Gcc-patches June 14, 2023, 7:06 a.m. UTC | #11
Sure, working on the V2 as well as the RV32 testing, will reply the bugfix PATCH once ready.

Pan

From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai>
Sent: Wednesday, June 14, 2023 3:01 PM
To: Robin Dapp <rdapp.gcc@gmail.com>; Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Robin Dapp <rdapp.gcc@gmail.com>; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split

>> Yeah sure, we need to be able to run tests only for specific targets.
>> Why does {riscv_vector} && {rv64} not work?
I am not sure. These testcases were added by kito long time ago.
Frankly, I am not familiar with GCC test framework.

I think the highest priority is to fix the "real" compiler bugs which I have noticed yesterday:
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/partial/multiple_rgroup_run-2.c execution test
FAIL: gcc.target/riscv/rvv/autovec/vls-vlmax/repeat_run-3.c -std=c99 -O3 -ftree-vectorize --param riscv-autovec-preference=fixed-vlmax execution test

@Li Pan could you verify whether your patch
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621610.html can fix these 2 issues?
If yes, please send V2 patch with append these information into patch log.


Thanks.
________________________________
juzhe.zhong@rivai.ai<mailto:juzhe.zhong@rivai.ai>

From: Robin Dapp<mailto:rdapp.gcc@gmail.com>
Date: 2023-06-14 14:52
To: juzhe.zhong@rivai.ai<mailto:juzhe.zhong@rivai.ai>; pan2.li<mailto:pan2.li@intel.com>; gcc-patches<mailto:gcc-patches@gcc.gnu.org>
CC: rdapp.gcc<mailto:rdapp.gcc@gmail.com>; jeffreyalaw<mailto:jeffreyalaw@gmail.com>; yanzhang.wang<mailto:yanzhang.wang@intel.com>; kito.cheng<mailto:kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split
Yes, I agree with the general assessment (and didn't mean to insinuate
that the FAILs are compiler's or a fault of the patch.

> So these 2 failures in RV32 are not the compile's bugs. I have seen:
> /* { dg-do run { target { { {riscv_vector} && {rv64} } } } } */ in
> these testcases which can not work to block execution in RV32 (Since
> such testcase only needs to be tested on RV64). I think this is the
> issue we need to figure out.

Yeah sure, we need to be able to run tests only for specific targets.
Why does {riscv_vector} && {rv64} not work?

For zvfh I'm testing something like the following:

proc check_effective_target_riscv_zvfh { } {
    if { ![istarget rv32*-*-*] && ![istarget rv64*-*-*] } then {
return 0;
    }

    if !check_effective_target_riscv_vector then {
return 0;
    }

    return [
[check_runtime riscv_check_zvfh {
    int main (void)
    {
asm ("vsetivli zero,8,e16,m1,ta,ma");
asm ("vfadd.vv %%v8,%%v8,%%v16" : : : "%%v8");
return 0;
    }
} "-march=rv64gcv_zvfh" ]
|| ... ]

Regards
Robin
  
Jeff Law June 14, 2023, 6:51 p.m. UTC | #12
On 6/13/23 20:31, juzhe.zhong@rivai.ai wrote:
> LGTM.
Similarly.  If I've interpreted the thread correctly, there aren't any 
issues created by this patch, though there are some existing issues that 
need to be addressed independently.  The patch itself is definitely the 
right thing to be doing.

I'd suggest going forward with the commit whenever it's convenient Pan.

Thanks,
Jeff
  
Li, Pan2 via Gcc-patches June 15, 2023, 1:07 a.m. UTC | #13
Committed, thanks Jeff and Juzhe, sorry for misleading.

Pan

-----Original Message-----
From: Jeff Law <jeffreyalaw@gmail.com> 
Sent: Thursday, June 15, 2023 2:51 AM
To: juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Robin Dapp <rdapp.gcc@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Align the predictor style for define_insn_and_split



On 6/13/23 20:31, juzhe.zhong@rivai.ai wrote:
> LGTM.
Similarly.  If I've interpreted the thread correctly, there aren't any issues created by this patch, though there are some existing issues that need to be addressed independently.  The patch itself is definitely the right thing to be doing.

I'd suggest going forward with the commit whenever it's convenient Pan.

Thanks,
Jeff
  

Patch

diff --git a/gcc/config/riscv/autovec-opt.md b/gcc/config/riscv/autovec-opt.md
index aef28e445e1..fb1b07205aa 100644
--- a/gcc/config/riscv/autovec-opt.md
+++ b/gcc/config/riscv/autovec-opt.md
@@ -37,9 +37,9 @@  (define_insn_and_split "@pred_single_widen_mul<any_extend:su><mode>"
 	      (match_operand:<V_DOUBLE_TRUNC> 4 "register_operand" "   vr,   vr"))
 	    (match_operand:VWEXTI 3 "register_operand"             "   vr,   vr"))
 	  (match_operand:VWEXTI 2 "vector_merge_operand"           "   vu,    0")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_vf2 (<CODE>, <MODE>mode);
@@ -132,9 +132,9 @@  (define_insn_and_split "*<optab>not<mode>"
 	(bitmanip_bitwise:VB
 	  (not:VB (match_operand:VB 2 "register_operand" " vr"))
 	  (match_operand:VB 1 "register_operand"         " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_not (<CODE>, <MODE>mode);
@@ -159,9 +159,9 @@  (define_insn_and_split "*n<optab><mode>"
 	  (any_bitwise:VB
 	    (match_operand:VB 1 "register_operand" " vr")
 	    (match_operand:VB 2 "register_operand" " vr"))))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_n (<CODE>, <MODE>mode);
@@ -346,9 +346,9 @@  (define_insn_and_split "*v<any_shiftrt:optab><any_extend:optab>trunc<mode>"
         (match_operand:VWEXTI 1 "register_operand"                 " vr,vr")
 	(any_extend:VWEXTI
           (match_operand:<V_DOUBLE_TRUNC> 2 "vector_shift_operand" " vr,vk")))))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
 {
   insn_code icode = code_for_pred_narrow (<any_shiftrt:CODE>, <MODE>mode);
@@ -364,9 +364,9 @@  (define_insn_and_split "*<any_shiftrt:optab>trunc<mode>"
       (any_shiftrt:VWEXTI
         (match_operand:VWEXTI 1 "register_operand"           " vr")
 	(match_operand:<VEL> 2 "csr_operand"                 " rK"))))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
 {
   operands[2] = gen_lowpart (Pmode, operands[2]);
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index eadc2c5b595..c23a625afe1 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -155,9 +155,9 @@  (define_insn_and_split "<optab><mode>3"
     (any_shift:VI
      (match_operand:VI 1 "register_operand"    " vr")
      (match_operand:<VEL> 2 "csr_operand"      " rK")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
 {
   operands[2] = gen_lowpart (Pmode, operands[2]);
@@ -180,9 +180,9 @@  (define_insn_and_split "v<optab><mode>3"
     (any_shift:VI
      (match_operand:VI 1 "register_operand"     " vr,vr")
      (match_operand:VI 2 "vector_shift_operand" " vr,vk")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
 {
   riscv_vector::emit_vlmax_insn (code_for_pred (<CODE>, <MODE>mode),
@@ -205,9 +205,9 @@  (define_insn_and_split "<optab><mode>3"
   [(set (match_operand:VB 0 "register_operand"                 "=vr")
 	(any_bitwise:VB (match_operand:VB 1 "register_operand" " vr")
 			(match_operand:VB 2 "register_operand" " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred (<CODE>, <MODE>mode);
@@ -227,9 +227,9 @@  (define_insn_and_split "<optab><mode>3"
 (define_insn_and_split "one_cmpl<mode>2"
   [(set (match_operand:VB 0 "register_operand"         "=vr")
 	(not:VB (match_operand:VB 1 "register_operand" " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
   {
     insn_code icode = code_for_pred_not (<MODE>mode);
@@ -366,9 +366,9 @@  (define_insn_and_split "<optab><v_double_trunc><mode>2"
   [(set (match_operand:VWEXTI 0 "register_operand" "=&vr")
     (any_extend:VWEXTI
      (match_operand:<V_DOUBLE_TRUNC> 1 "register_operand" "vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
 {
   insn_code icode = code_for_pred_vf2 (<CODE>, <MODE>mode);
@@ -409,9 +409,9 @@  (define_insn_and_split "trunc<mode><v_double_trunc>2"
   [(set (match_operand:<V_DOUBLE_TRUNC> 0 "register_operand" "=vr")
     (truncate:<V_DOUBLE_TRUNC>
      (match_operand:VWEXTI 1 "register_operand"              " vr")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && can_create_pseudo_p ()"
   "#"
-  "&& can_create_pseudo_p ()"
+  "&& 1"
   [(const_int 0)]
 {
   insn_code icode = code_for_pred_trunc (<MODE>mode);