public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>
To: "Robin Dapp" <rdapp.gcc@gmail.com>,
	 gcc-patches <gcc-patches@gcc.gnu.org>,
	 kito.cheng <kito.cheng@gmail.com>,
	 Kito.cheng <Kito.cheng@sifive.com>,  palmer <palmer@dabbelt.com>,
	 collison <collison@rivosinc.com>,
	 jeffreyalaw <jeffreyalaw@gmail.com>
Subject: Re: [PATCH] riscv: Allow vector constants in riscv_const_insns.
Date: Thu, 4 May 2023 13:07:46 +0800	[thread overview]
Message-ID: <4B555E0D49C3178B+2023050413074589268520@rivai.ai> (raw)
In-Reply-To: <46ca12b2-8ac6-030e-92dc-6b71ab2d4ee8@gmail.com>

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

This ideal of this patch looks good to me.
But I think this patch should be able to handle more cases (not only -16 ~ 15) in case of CONST_VECTOR initialization.

Case 1 (Other constant value that is not -16 ~ 15):
void vmv_m##VAL (TYPE dst[], int n) \
{                                                     \
    for (int i = 0; i < n; i++)                         \
      dst[i] = 100; \
  }

I guess for const_vector:100 is not optimal currently so far, I think you may try (and add testcases).
Such code can be:

Codegen 1:                            Codegen 2:
li a5,100                                  vlse.v v24, (a5), zero ;; a5 address memory has the value of 100.
vmv.v.x v1, a5

I am not sure codegen 1 or codegen 2, which one is better. I think you can decide it.
But my idea is that I think this patch should not only handle he constant value of -16 ~ 15, but also other constant value should be handled and tested in this patch.

Case 2 (Constant value within 32bit for INT64 in RV32 system):

This is a special case:

void vmv_i64 (TYPE dst[], int n)
{                                                    
    for (int i = 0; i < n; i++)                        
      dst[i] = 0xAAAAAAAA;
 }

In this case, the Codegen should be similiar with Case 1 since each scalar register can hold the whole constant value.


Case 3 (Constant value over 32bit for INT64 in RV32 system):

This is a special case:

void vmv_i64 (TYPE dst[], int n)
{                                                    
    for (int i = 0; i < n; i++)                        
      dst[i] = 0xAAAAAAAAA;
 }

In this case, since each scalar register can only hold 32bit value that is not the whole constant value (0xAAAAAAAAA)
I think in this case, we can only use vlse.v...

Would you refine this patch more? Thanks.


juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-04-29 00:10
To: gcc-patches; Kito Cheng; Kito.cheng; palmer; juzhe.zhong@rivai.ai; Michael Collison; jeffreyalaw
Subject: [PATCH] riscv: Allow vector constants in riscv_const_insns.
Hi,
 
I figured I'm going to start sending some patches that build on top
of the upcoming RISC-V autovectorization.  This one is obviously
not supposed to be installed before the basic support lands but
it's small enough that it shouldn't hurt to send it now.
 
This patch allows vector constants in riscv_const_insns in order
for them to be properly recognized as immediate operands such that
we can emit vmv.v.i instructions via autovec.
 
Bootstrapped and regtested on riscv32gcv and riscv64gcv.
 
Regards
Robin
 
--
 
gcc/ChangeLog:
 
* config/riscv/riscv.cc (riscv_const_insns): Add permissible
vector constants.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/autovec/vmv-imm.c: New test.
---
gcc/config/riscv/riscv.cc                     |  10 +-
.../gcc.target/riscv/rvv/autovec/vmv-imm.c    | 109 ++++++++++++++++++
2 files changed, 118 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm.c
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index eb7364ca110..6f9c6743028 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -1228,7 +1228,15 @@ riscv_const_insns (rtx x)
     case CONST_DOUBLE:
     case CONST_VECTOR:
       /* We can use x0 to load floating-point zero.  */
-      return x == CONST0_RTX (GET_MODE (x)) ? 1 : 0;
+      if (x == CONST0_RTX (GET_MODE (x)))
+ return 1;
+      /* Constants from -16 to 15 can be loaded with vmv.v.i.
+ The Wc0, Wc1 constraints are already covered by the
+ vi constraint so we do not need to check them here
+ separately.  */
+      else if (TARGET_VECTOR && satisfies_constraint_vi (x))
+ return 1;
+      return 0;
     case CONST:
       /* See if we can refer to X directly.  */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm.c
new file mode 100644
index 00000000000..42ca56d4b5c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vmv-imm.c
@@ -0,0 +1,109 @@
+/* { dg-do run } */
+/* { dg-additional-options "-std=c99 --param=riscv-autovec-preference=scalable -fno-vect-cost-model -fno-builtin --save-temps" } */
+
+#include <stdint.h>
+#include <assert.h>
+
+#define VMV_POS(TYPE,VAL) \
+  __attribute__ ((noipa))                               \
+  void vmv_##VAL (TYPE dst[], int n) \
+  {                                                     \
+    for (int i = 0; i < n; i++)                         \
+      dst[i] = VAL; \
+  }
+
+#define VMV_NEG(TYPE,VAL) \
+  __attribute__ ((noipa))                               \
+  void vmv_m##VAL (TYPE dst[], int n) \
+  {                                                     \
+    for (int i = 0; i < n; i++)                         \
+      dst[i] = -VAL; \
+  }
+
+#define TEST_ALL() \
+VMV_NEG(int8_t,16) \
+VMV_NEG(int8_t,15)    \
+VMV_NEG(int8_t,14)    \
+VMV_NEG(int8_t,13)    \
+VMV_NEG(int16_t,12)     \
+VMV_NEG(int16_t,11)     \
+VMV_NEG(int16_t,10)     \
+VMV_NEG(int16_t,9) \
+VMV_NEG(int32_t,8) \
+VMV_NEG(int32_t,7) \
+VMV_NEG(int32_t,6) \
+VMV_NEG(int32_t,5) \
+VMV_NEG(int64_t,4) \
+VMV_NEG(int64_t,3) \
+VMV_NEG(int64_t,2) \
+VMV_NEG(int64_t,1) \
+VMV_POS(uint8_t,0) \
+VMV_POS(uint8_t,1) \
+VMV_POS(uint8_t,2) \
+VMV_POS(uint8_t,3) \
+VMV_POS(uint16_t,4) \
+VMV_POS(uint16_t,5) \
+VMV_POS(uint16_t,6) \
+VMV_POS(uint16_t,7) \
+VMV_POS(uint32_t,8) \
+VMV_POS(uint32_t,9) \
+VMV_POS(uint32_t,10) \
+VMV_POS(uint32_t,11) \
+VMV_POS(uint64_t,12) \
+VMV_POS(uint64_t,13) \
+VMV_POS(uint64_t,14) \
+VMV_POS(uint64_t,15)
+
+TEST_ALL()
+
+#define SZ 32
+
+#define TEST_POS(TYPE,VAL) \
+  TYPE a##TYPE##VAL[SZ];   \
+  vmv_##VAL (a##TYPE##VAL, SZ);   \
+  for (int i = 0; i < SZ; i++)   \
+    assert (a##TYPE##VAL[i] == VAL);
+
+#define TEST_NEG(TYPE,VAL) \
+  TYPE am##TYPE##VAL[SZ];   \
+  vmv_m##VAL (am##TYPE##VAL, SZ); \
+  for (int i = 0; i < SZ; i++)   \
+    assert (am##TYPE##VAL[i] == -VAL);
+
+int main ()
+{
+  TEST_NEG(int8_t, 16)
+  TEST_NEG(int8_t, 15)
+  TEST_NEG(int8_t, 14)
+  TEST_NEG(int8_t, 13)
+  TEST_NEG(int16_t, 12)
+  TEST_NEG(int16_t, 11)
+  TEST_NEG(int16_t, 10)
+  TEST_NEG(int16_t, 9)
+  TEST_NEG(int32_t, 8)
+  TEST_NEG(int32_t, 7)
+  TEST_NEG(int32_t, 6)
+  TEST_NEG(int32_t, 5)
+  TEST_NEG(int64_t, 4)
+  TEST_NEG(int64_t, 3)
+  TEST_NEG(int64_t, 2)
+  TEST_NEG(int64_t, 1)
+  TEST_POS(uint8_t, 0)
+  TEST_POS(uint8_t, 1)
+  TEST_POS(uint8_t, 2)
+  TEST_POS(uint8_t, 3)
+  TEST_POS(uint16_t, 4)
+  TEST_POS(uint16_t, 5)
+  TEST_POS(uint16_t, 6)
+  TEST_POS(uint16_t, 7)
+  TEST_POS(uint32_t, 8)
+  TEST_POS(uint32_t, 9)
+  TEST_POS(uint32_t, 10)
+  TEST_POS(uint32_t, 11)
+  TEST_POS(uint64_t, 12)
+  TEST_POS(uint64_t, 13)
+  TEST_POS(uint64_t, 14)
+  TEST_POS(uint64_t, 15)
+}
+
+/* { dg-final { scan-assembler-times "vmv.v.i" 32 } } */
-- 
2.40.0
 

  parent reply	other threads:[~2023-05-04  5:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-28 16:10 Robin Dapp
2023-04-28 20:34 ` Jeff Law
2023-05-04  5:07 ` juzhe.zhong [this message]
2023-05-06 20:11   ` Jeff Law
2023-05-07  0:09     ` 钟居哲
2023-05-11 12:47       ` [PATCH v2] RISC-V: " Robin Dapp
2023-05-11 14:15         ` Kito Cheng

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=4B555E0D49C3178B+2023050413074589268520@rivai.ai \
    --to=juzhe.zhong@rivai.ai \
    --cc=Kito.cheng@sifive.com \
    --cc=collison@rivosinc.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    --cc=kito.cheng@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=rdapp.gcc@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).