public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick O'Neill <patrick@rivosinc.com>
To: Kito Cheng <kito.cheng@gmail.com>,
	"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>
Cc: "demin.han" <demin.han@starfivetech.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] RISC-V: Fix uninitialized and redundant use of which_alternative
Date: Thu, 27 Jul 2023 13:46:23 -0700	[thread overview]
Message-ID: <b6803864-3810-1534-e771-9f5c644dcbe6@rivosinc.com> (raw)
In-Reply-To: <CA+yXCZCQ4RX32_B5XcTD1z50BgCv5WCPmXFwv1TOsWpjr3DNkA@mail.gmail.com>

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

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.zhong@rivai.ai
> <juzhe.zhong@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.zhong@rivai.ai
>>
>> From: demin.han
>> Date: 2023-07-27 17:48
>> To:gcc-patches@gcc.gnu.org
>> CC:kito.cheng@gmail.com;juzhe.zhong@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.han@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
>>
>>

  reply	other threads:[~2023-07-27 20:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-27  9:48 demin.han
2023-07-27 10:22 ` juzhe.zhong
2023-07-27 11:57   ` Kito Cheng
2023-07-27 20:46     ` Patrick O'Neill [this message]
2023-07-28  2:55       ` Demin Han
2023-07-28 16:42         ` Patrick O'Neill
2023-07-28 21:55           ` Jeff Law
     [not found]     ` <9819777D4BE5D5B8+4EB8A5B1-0546-4283-910B-CD4EA60B1F7A@rivai.ai>
2023-07-28 16:37       ` [Committed] RISC-V: Specify -mabi in rv64 autovec testcase Patrick O'Neill
2023-07-28 16:40       ` [PATCH] RISC-V: Fix uninitialized and redundant use of which_alternative Patrick O'Neill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b6803864-3810-1534-e771-9f5c644dcbe6@rivosinc.com \
    --to=patrick@rivosinc.com \
    --cc=demin.han@starfivetech.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=juzhe.zhong@rivai.ai \
    --cc=kito.cheng@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).