public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-6268] RISC-V: xtheadfmemidx: Disable if xtheadmemidx is not available
@ 2023-12-07  9:33 Christoph Mテシllner
  0 siblings, 0 replies; only message in thread
From: Christoph Mテシllner @ 2023-12-07  9:33 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:8c09c73adf4c537876e8ef418378d6581b768a64

commit r14-6268-g8c09c73adf4c537876e8ef418378d6581b768a64
Author: Christoph Müllner <christoph.muellner@vrull.eu>
Date:   Tue Dec 5 01:00:11 2023 +0100

    RISC-V: xtheadfmemidx: Disable if xtheadmemidx is not available
    
    XTheadMemIdx provides register-register offsets for GP register
    loads/stores.  XTheadFMemIdx does the same for FP registers.
    
    We've observed an issue with XTheadFMemIdx-only builds, where FP
    registers have been promoted to GP registers:
    
    (insn 26 22 51 (set (reg:DF 15 a5 [orig:136 <retval> ] [136])
            (mem/u:DF (plus:DI (reg/f:DI 15 a5 [141])
                    (reg:DI 10 a0 [144])) [1 CSWTCH.2[_10]+0 S8 A64])) 217 {*movdf_hardfloat_rv64}
         (expr_list:REG_DEAD (reg:DI 10 a0 [144])
            (nil)))
    
    This results in the following assembler error:
      Assembler messages:
      Error: unrecognized opcode `th.lrd a5,a5,a0,0', extension `xtheadmemidx' required
    
    There seems to be a (reasonable) assumption, that addressing modes
    for FP registers are compatible with those of GP registers.
    
    We already ran into a similar issue during development of the
    XTheadFMemIdx support patch, where we could trace the issue down to
    the optimization splitters.  Back then we simply disabled them in case
    XTheadMemIdx is not available.  But as it turned out, that was not
    enough.
    
    To ensure, we won't see such issues anymore, let's make the support
    for XTheadFMemIdx depend on XTheadMemIdx.  I.e., if only XTheadFMemIdx
    is available, then no instructions of this extension will be emitted.
    
    While this looks a bit drastic at first view, it is the best practical
    solution since XTheadFMemIdx without XTheadMemIdx does not exist in real
    hardware and would be an odd thing to do.
    
    gcc/ChangeLog:
    
            * config/riscv/thead.cc (th_memidx_classify_address_index):
            Require TARGET_XTHEADMEMIDX for FP modes.
            * config/riscv/thead.md: Require TARGET_XTHEADMEMIDX for all
            XTheadFMemIdx pattern.
    
    gcc/testsuite/ChangeLog:
    
            * gcc.target/riscv/xtheadfmemidx-without-xtheadmemidx.c: New test.
    
    Reported-by: Jin Ma <jinma@linux.alibaba.com>
    Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>

Diff:
---
 gcc/config/riscv/thead.cc                          |  3 +-
 gcc/config/riscv/thead.md                          | 19 ++++++-----
 .../riscv/xtheadfmemidx-without-xtheadmemidx.c     | 39 ++++++++++++++++++++++
 3 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc
index bd9af7ecd60..20353995931 100644
--- a/gcc/config/riscv/thead.cc
+++ b/gcc/config/riscv/thead.cc
@@ -603,7 +603,8 @@ th_memidx_classify_address_index (struct riscv_address_info *info, rtx x,
 {
   /* Ensure that the mode is supported.  */
   if (!(TARGET_XTHEADMEMIDX && is_memidx_mode (mode))
-      && !(TARGET_XTHEADFMEMIDX && is_fmemidx_mode (mode)))
+      && !(TARGET_XTHEADMEMIDX
+	   && TARGET_XTHEADFMEMIDX && is_fmemidx_mode (mode)))
     return false;
 
   if (GET_CODE (x) != PLUS)
diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
index 2babfafb23c..186ca468875 100644
--- a/gcc/config/riscv/thead.md
+++ b/gcc/config/riscv/thead.md
@@ -822,11 +822,19 @@
 )
 
 ;; XTheadFMemIdx
+;; Note, that we might get GP registers in FP-mode (reg:DF a2)
+;; which cannot be handled by the XTheadFMemIdx instructions.
+;; This might even happend after register allocation.
+;; We could implement splitters that undo the combiner results
+;; if "after_reload && !HARDFP_REG_P (operands[0])", but this
+;; raises even more questions (e.g. split into what?).
+;; So let's solve this by simply requiring XTheadMemIdx
+;; which provides the necessary instructions to cover this case.
 
 (define_insn "*th_fmemidx_movsf_hardfloat"
   [(set (match_operand:SF 0 "nonimmediate_operand" "=f,th_m_mir,f,th_m_miu")
 	(match_operand:SF 1 "move_operand"         " th_m_mir,f,th_m_miu,f"))]
-  "TARGET_HARD_FLOAT && TARGET_XTHEADFMEMIDX
+  "TARGET_HARD_FLOAT && TARGET_XTHEADFMEMIDX && TARGET_XTHEADMEMIDX
    && (register_operand (operands[0], SFmode)
        || reg_or_0_operand (operands[1], SFmode))"
   { return riscv_output_move (operands[0], operands[1]); }
@@ -837,6 +845,7 @@
   [(set (match_operand:DF 0 "nonimmediate_operand" "=f,th_m_mir,f,th_m_miu")
 	(match_operand:DF 1 "move_operand"         " th_m_mir,f,th_m_miu,f"))]
   "TARGET_64BIT && TARGET_DOUBLE_FLOAT && TARGET_XTHEADFMEMIDX
+   && TARGET_XTHEADMEMIDX
    && (register_operand (operands[0], DFmode)
        || reg_or_0_operand (operands[1], DFmode))"
   { return riscv_output_move (operands[0], operands[1]); }
@@ -845,14 +854,6 @@
 
 ;; XTheadFMemIdx optimizations
 ;; Similar like XTheadMemIdx optimizations, but less cases.
-;; Note, that we might get GP registers in FP-mode (reg:DF a2)
-;; which cannot be handled by the XTheadFMemIdx instructions.
-;; This might even happend after register allocation.
-;; We could implement splitters that undo the combiner results
-;; if "after_reload && !HARDFP_REG_P (operands[0])", but this
-;; raises even more questions (e.g. split into what?).
-;; So let's solve this by simply requiring XTheadMemIdx
-;; which provides the necessary instructions to cover this case.
 
 (define_insn_and_split "*th_fmemidx_I_a"
   [(set (match_operand:TH_M_NOEXTF 0 "register_operand" "=f")
diff --git a/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-without-xtheadmemidx.c b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-without-xtheadmemidx.c
new file mode 100644
index 00000000000..c5502390cca
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-without-xtheadmemidx.c
@@ -0,0 +1,39 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Og" } } */
+/* { dg-options "-march=rv64gc_xtheadfmemidx" { target { rv64 } } } */
+/* { dg-options "-march=rv32gc_xtheadfmemidx" { target { rv32 } } } */
+
+typedef union {
+  double v;
+  unsigned w;
+} my_t;
+
+double z;
+
+double foo (int i, int j)
+{
+
+  if (j)
+    {
+      switch (i)
+	{
+	case 0:
+	  return 1;
+	case 1:
+	  return 0;
+	case 2:
+	  return 3.0;
+	}
+    }
+
+  if (i == 1)
+    {
+      my_t u;
+      u.v = z;
+      u.w = 1;
+      z = u.v;
+    }
+  return z;
+}
+
+/* { dg-final { scan-assembler-not "th.lrd\t" } } */

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-12-07  9:33 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-07  9:33 [gcc r14-6268] RISC-V: xtheadfmemidx: Disable if xtheadmemidx is not available Christoph Mテシllner

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).