Re: [PATCH] RISC-V: Fix uninitialized and redundant use of which_alternative

Patrick O'Neill Fri, 28 Jul 2023 09:40:45 -0700

Thanks!
Here's the comitted patch:
https://inbox.sourceware.org/gcc-patches/20230728163758.377962-1-patr...@rivosinc.com/T/#u


On 7/27/23 15:11, juzhe.zhong wrote:

LGTM.Thanks. You can go ahead commit it.
---- Replied Message ----
From    Patrick O'Neill<patr...@rivosinc.com> <mailto:patr...@rivosinc.com>
Date    07/28/2023 04:46
To      Kito Cheng<kito.ch...@gmail.com> <mailto:kito.ch...@gmail.com>,
juzhe.zh...@rivai.ai<juzhe.zh...@rivai.ai> <mailto:juzhe.zh...@rivai.ai>

Cc demin.han<demin....@starfivetech.com><mailto:demin....@starfivetech.com>,

gcc-patches<gcc-patches@gcc.gnu.org> <mailto:gcc-patches@gcc.gnu.org>

Subject Re: [PATCH] RISC-V: Fix uninitialized and redundant use ofwhich_alternative


The newly added testcase fails on rv32 targets with this message:
FAIL: gcc.target/riscv/rvv/autovec/madd-split2-1.c -O3 -ftree-vectorize (test 
for excess errors)

verbose log:
compiler exited with status 1
output is:
cc1: error: ABI requires '-march=rv32'

Something like this appears to fix the issue:

diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c
b/gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c
index 14a9802667e..e10a9e9d0f5 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c
@@ -1,5 +1,5 @@
  /* { dg-do compile } */
-/* { dg-options "-march=rv64gcv_zvl256b -O3 -fno-cprop-registers -fno-dce --param 
riscv-autovec-preference=scalable" } */
+/* { dg-options "-march=rv64gcv_zvl256b -mabi=lp64d -O3
-fno-cprop-registers -fno-dce --param riscv-autovec-preference=scalable"
  } */

long

  foo (long *__restrict a, long *__restrict b, long n)

On 7/27/23 04:57, Kito Cheng via Gcc-patches wrote:

My first impression is those emit_insn (gen_rtx_SET()) seems
necessary, but I got the point after I checked vector.md :P

Committed to trunk, thanks :)


On Thu, Jul 27, 2023 at 6:23 pmjuzhe.zh...@rivai.ai
<juzhe.zh...@rivai.ai>  wrote:

Oh, YES.

Thanks for fixing it. It makes sense since the ternary operations in "vector.md"
generate "vmv.v.v" according to RA.

Thanks for fixing it.

@kito: Could you confirm it? If it's ok to you, commit it for Han (I am lazy to 
commit patches :).



juzhe.zh...@rivai.ai

From: demin.han
Date: 2023-07-27 17:48
To:gcc-patches@gcc.gnu.org
CC:kito.ch...@gmail.com;juzhe.zh...@rivai.ai
Subject: [PATCH] RISC-V: Fix uninitialized and redundant use of 
which_alternative
When pass split2 starts, which_alternative is random depending on
last set of certain pass.

Even initialized, the generated movement is redundant.
The movement can be generated by assembly output template.

Signed-off-by: demin.han<demin....@starfivetech.com>

gcc/ChangeLog:

* config/riscv/autovec.md: Delete which_alternative use in split

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/autovec/madd-split2-1.c: New test.

---
gcc/config/riscv/autovec.md                         | 12 ------------
.../gcc.target/riscv/rvv/autovec/madd-split2-1.c    | 13 +++++++++++++
2 files changed, 13 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index d899922586a..b7ea3101f5a 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -1012,8 +1012,6 @@ (define_insn_and_split "*fma<VI:mode><P:mode>"
    [(const_int 0)]
    {
      riscv_vector::emit_vlmax_vsetvl (<VI:MODE>mode, operands[4]);
-    if (which_alternative == 2)
-      emit_insn (gen_rtx_SET (operands[0], operands[3]));
      rtx ops[] = {operands[0], operands[1], operands[2], operands[3], 
operands[0]};
      riscv_vector::emit_vlmax_ternary_insn (code_for_pred_mul_plus 
(<VI:MODE>mode),
    riscv_vector::RVV_TERNOP, ops, operands[4]);
@@ -1058,8 +1056,6 @@ (define_insn_and_split "*fnma<VI:mode><P:mode>"
    [(const_int 0)]
    {
      riscv_vector::emit_vlmax_vsetvl (<VI:MODE>mode, operands[4]);
-    if (which_alternative == 2)
-      emit_insn (gen_rtx_SET (operands[0], operands[3]));
      rtx ops[] = {operands[0], operands[1], operands[2], operands[3], 
operands[0]};
      riscv_vector::emit_vlmax_ternary_insn (code_for_pred_minus_mul 
(<VI:MODE>mode),
         riscv_vector::RVV_TERNOP, ops, operands[4]);
@@ -1102,8 +1098,6 @@ (define_insn_and_split "*fma<VF:mode><P:mode>"
    [(const_int 0)]
    {
      riscv_vector::emit_vlmax_vsetvl (<VF:MODE>mode, operands[4]);
-    if (which_alternative == 2)
-      emit_insn (gen_rtx_SET (operands[0], operands[3]));
      rtx ops[] = {operands[0], operands[1], operands[2], operands[3], 
operands[0]};
      riscv_vector::emit_vlmax_fp_ternary_insn (code_for_pred_mul (PLUS, 
<VF:MODE>mode),
       riscv_vector::RVV_TERNOP, ops, operands[4]);
@@ -1148,8 +1142,6 @@ (define_insn_and_split "*fnma<VF:mode><P:mode>"
    [(const_int 0)]
    {
      riscv_vector::emit_vlmax_vsetvl (<VF:MODE>mode, operands[4]);
-    if (which_alternative == 2)
-      emit_insn (gen_rtx_SET (operands[0], operands[3]));
      rtx ops[] = {operands[0], operands[1], operands[2], operands[3], 
operands[0]};
      riscv_vector::emit_vlmax_fp_ternary_insn (code_for_pred_mul_neg (PLUS, 
<VF:MODE>mode),
       riscv_vector::RVV_TERNOP, ops, operands[4]);
@@ -1194,8 +1186,6 @@ (define_insn_and_split "*fms<VF:mode><P:mode>"
    [(const_int 0)]
    {
      riscv_vector::emit_vlmax_vsetvl (<VF:MODE>mode, operands[4]);
-    if (which_alternative == 2)
-      emit_insn (gen_rtx_SET (operands[0], operands[3]));
      rtx ops[] = {operands[0], operands[1], operands[2], operands[3], 
operands[0]};
      riscv_vector::emit_vlmax_fp_ternary_insn (code_for_pred_mul (MINUS, 
<VF:MODE>mode),
       riscv_vector::RVV_TERNOP, ops, operands[4]);
@@ -1242,8 +1232,6 @@ (define_insn_and_split "*fnms<VF:mode><P:mode>"
    [(const_int 0)]
    {
      riscv_vector::emit_vlmax_vsetvl (<VF:MODE>mode, operands[4]);
-    if (which_alternative == 2)
-      emit_insn (gen_rtx_SET (operands[0], operands[3]));
      rtx ops[] = {operands[0], operands[1], operands[2], operands[3], 
operands[0]};
      riscv_vector::emit_vlmax_fp_ternary_insn (code_for_pred_mul_neg (MINUS, 
<VF:MODE>mode),
       riscv_vector::RVV_TERNOP, ops, operands[4]);
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c 
b/gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c
new file mode 100644
index 00000000000..14a9802667e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/madd-split2-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zvl256b -O3 -fno-cprop-registers -fno-dce --param 
riscv-autovec-preference=scalable" } */
+
+long
+foo (long *__restrict a, long *__restrict b, long n)
+{
+  long i;
+  for (i = 0; i < n; ++i)
+    a[i] = b[i] + i * 8;
+  return a[1];
+}
+
+/* { dg-final { scan-assembler-times {\tvmv1r\.v} 1 } } */
--
2.41.0

Re: [PATCH] RISC-V: Fix uninitialized and redundant use of which_alternative

Reply via email to