Re: RE: [PATCH v1] RISC-V: Bugfix for legitimize move when get vec mode in zve32f

["juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>]

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

I see.

Is it possible to block this situation in other place ? that is, make the situation you said never reach here.

Or handle this situation with this following approach:

if (get_vector_mode doesn't exist such mode)

1. extract V2SI -> lowpart of the DI reg.
2. extract V2SI -> highpart of the DI reg.
3. scalar move DI reg to DF reg.

This patch just bypass the situation (return false) you mention is quite unsafe here.
As I remembered, I add this code to handle some ICE due to "return false" here.



juzhe.zhong@rivai.ai
 
From: Li, Pan2
Date: 2023-11-30 15:42
To: juzhe.zhong@rivai.ai; gcc-patches
CC: Wang, Yanzhang; kito.cheng
Subject: RE: RE: [PATCH v1] RISC-V: Bugfix for legitimize move when get vec mode in zve32f
Take this file riscv-gnu-toolchain/newlib/newlib/libc/stdlib/mprec.c for example, the arguments and/or related local variables list as below
 
riscv_legitimize_move
  mode = E_DFmode
  dest = (reg:DF 144 [ <retval> ])
  src = (subreg:DF (reg:V2SI 170) 0)
 
nunits = 1
smode = {m_mode = E_DFmode}
 
Pan
 
From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai> 
Sent: Thursday, November 30, 2023 3:33 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: RE: [PATCH v1] RISC-V: Bugfix for legitimize move when get vec mode in zve32f
 
What it the RTX of the operand ?
 


juzhe.zhong@rivai.ai
 
From: Li, Pan2
Date: 2023-11-30 15:31
To: juzhe.zhong@rivai.ai; gcc-patches
CC: Wang, Yanzhang; kito.cheng
Subject: RE: [PATCH v1] RISC-V: Bugfix for legitimize move when get vec mode in zve32f
> Why does get_vector_mode doesn't exist a vector mode ?
 
Because we set the zve32f here, but try to get_vect_mode with E_V1DFmode.
According to the ISA, FP64 is not support when zve32F.
 
Pan
 
From: juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai> 
Sent: Thursday, November 30, 2023 3:24 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Li, Pan2 <pan2.li@intel.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Bugfix for legitimize move when get vec mode in zve32f
 
Why does get_vector_mode doesn't exist a vector mode ?
 
It must exist a vector mode, otherwise, it will cause ICE in other situations.
 


juzhe.zhong@rivai.ai
 
From: pan2.li
Date: 2023-11-30 15:21
To: gcc-patches
CC: juzhe.zhong; pan2.li; yanzhang.wang; kito.cheng
Subject: [PATCH v1] RISC-V: Bugfix for legitimize move when get vec mode in zve32f
From: Pan Li <pan2.li@intel.com>
 
When require mode after get_vec_mode in riscv_legitimize_move,
there will be precondition that the mode is exists. Or we will
have E_VOIDMode and of course have ICE when required.
 
Typically we should first check the mode exists or not before
require, or more friendly like leverage exist (U *mode) to get
the expected mode if exists and unchanged if not.
 
This patch would like to fix this by exist (U *mode) for requiring
a mode after get_vec_mode.
 
PR target/112743
 
gcc/ChangeLog:
 
* config/riscv/riscv.cc (riscv_legitimize_move): Take the
exist (U *mode) instead of directly require ().
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/base/pr112743-2.c: New test.
 
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/config/riscv/riscv.cc                     | 47 ++++++++++-------
.../gcc.target/riscv/rvv/base/pr112743-2.c    | 52 +++++++++++++++++++
2 files changed, 79 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr112743-2.c
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index a4fc858fb50..19413b2c976 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -2615,32 +2615,39 @@ riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
  smode = SImode;
  nunits = nunits * 2;
}
-      vmode = riscv_vector::get_vector_mode (smode, nunits).require ();
-      rtx v = gen_lowpart (vmode, SUBREG_REG (src));
-      for (unsigned int i = 0; i < num; i++)
+      opt_machine_mode opt_mode = riscv_vector::get_vector_mode (smode, nunits);
+
+      if (opt_mode.exists (&vmode))
{
-   rtx result;
-   if (num == 1)
-     result = dest;
-   else if (i == 0)
-     result = gen_lowpart (smode, dest);
-   else
-     result = gen_reg_rtx (smode);
-   riscv_vector::emit_vec_extract (result, v, index + i);
+   rtx v = gen_lowpart (vmode, SUBREG_REG (src));
-   if (i == 1)
+   for (unsigned int i = 0; i < num; i++)
    {
-       rtx tmp
- = expand_binop (Pmode, ashl_optab, gen_lowpart (Pmode, result),
- gen_int_mode (32, Pmode), NULL_RTX, 0,
- OPTAB_DIRECT);
-       rtx tmp2 = expand_binop (Pmode, ior_optab, tmp, dest, NULL_RTX, 0,
-        OPTAB_DIRECT);
-       emit_move_insn (dest, tmp2);
+       rtx result;
+       if (num == 1)
+ result = dest;
+       else if (i == 0)
+ result = gen_lowpart (smode, dest);
+       else
+ result = gen_reg_rtx (smode);
+
+       riscv_vector::emit_vec_extract (result, v, index + i);
+
+       if (i == 1)
+ {
+   rtx tmp = expand_binop (Pmode, ashl_optab,
+   gen_lowpart (Pmode, result),
+   gen_int_mode (32, Pmode), NULL_RTX, 0,
+   OPTAB_DIRECT);
+   rtx tmp2 = expand_binop (Pmode, ior_optab, tmp, dest,
+    NULL_RTX, 0,
+    OPTAB_DIRECT);
+   emit_move_insn (dest, tmp2);
+ }
    }
+   return true;
}
-      return true;
     }
   /* Expand
        (set (reg:QI target) (mem:QI (address)))
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr112743-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr112743-2.c
new file mode 100644
index 00000000000..fdb35fd70f2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr112743-2.c
@@ -0,0 +1,52 @@
+/* Test that we do not have ice when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zve32f_zvfh_zfh -mabi=lp64 -O2" } */
+
+#include <sys/types.h>
+
+union double_union
+{
+  double d;
+  __uint32_t i[2];
+};
+
+#define word0(x)  (x.i[1])
+#define word1(x)  (x.i[0])
+
+#define P         53
+#define Exp_shift 20
+#define Exp_msk1  ((__uint32_t)0x100000L)
+#define Exp_mask  ((__uint32_t)0x7ff00000L)
+
+double ulp (double _x)
+{
+  union double_union x, a;
+  register int L;
+
+  x.d = _x;
+  L = (word0 (x) & Exp_mask) - (P - 1) * Exp_msk1;
+
+  if (L > 0)
+    {
+      L |= Exp_msk1 >> 4;
+      word0 (a) = L;
+      word1 (a) = 0;
+    }
+  else
+    {
+      L = -L >> Exp_shift;
+      if (L < Exp_shift)
+ {
+   word0 (a) = 0x80000 >> L;
+   word1 (a) = 0;
+ }
+      else
+ {
+   word0 (a) = 0;
+   L -= Exp_shift;
+   word1 (a) = L >= 31 ? 1 : 1 << (31 - L);
+ }
+    }
+
+  return a.d;
+}
-- 
2.34.1