xtensa: Renovate support for load/store of hardware FP register with pre/post-modify side-effect

Message ID 7fa7d572-fa28-4176-a815-63521260daea@yahoo.co.jp
State New
Headers
Series xtensa: Renovate support for load/store of hardware FP register with pre/post-modify side-effect |

Commit Message

Takayuki 'January June' Suwa Sept. 13, 2025, 10:43 a.m. UTC
  The Xtensa ISA has hardware FP register load/store instructions that have
pre- or post-modify side effects (LSIU/SSIU or LSIP/SSIP machine instruc-
tions) depending on the configuration and currently these are defined using
parallel RTX, but in fact they do not work at all:

   "An instruction that can be represented with an embedded side effect
    could also be represented using parallel containing an additional set
    to describe how the address register is altered.  This is not done
    because machines that allow these operations at all typically allow
    them wherever a memory address is called for.  Describing them as
    additional parallel stores would require doubling the number of entries
    in the machine description."

   -- 13.16 Embedded Side-Effects on Addresses, gccint (the latest)

Looking back for the commit history, this has been the case since gcc 3,
and there was a similar description in gccint at the time, so there must
have been some reason that we cannot think of at the moment.

This patch re-implements support for the above instructions in a modern
manner, in other words, using appropriate embedded side-effect RTXes.

gcc/ChangeLog:

	* config/xtensa/constraints.md (U): Add condition that the address
	must have no side-effects.
	(S): New constraint to represent non-memory pool reference whose
	address has some side-effect.
	* config/xtensa/xtensa.h
	(HAVE_PRE_MODIFY_DISP, HAVE_POST_MODIFY_DISP): New macros.
	* config/xtensa/xtensa-protos.h (sideeffect_addr_mem_p):
	New function prototype.
	* config/xtensa/xtensa.cc (sideeffect_addr_mem_p):
	New function definition that returns true if the address of the
	specified mem RTX has a supported side-effect.
	(TARGET_LEGITIMATE_ADDRESS_P): Replace a tab with a whitespace
	(cosmetics).
	(xtensa_legitimate_address_p): Change to return true even if the
	address has a supported side-effect.
	(print_operand): Add a format to output a single character to be
	appended to the assembler mnemonic corresponding to the side-effect
	of the specified memory operand.
	(print_operand_address): Change to accept addresses even if they
	have side-effects.
	(pprint_operand, xtensa_output_literal): Replace fputs() of
	a string consisting of a single character with fputc() of that
	character.
	* config/xtensa/xtensa.md (*lsiu, *ssiu, *lsip, *ssip):
	Remove due to breakdown.
	(movsf_internal): Add templates for memory reference/store machine
	instructions with pre/post_modify properties.

--- >8 ---

(Notes to be worried about...)

There are two concerns I had while making this patch; one is for the
maintainers of this target, and the other is for the GCC developer gurus
- especially the maintainers of the RTL auto-inc-dec pass.

(i.) There might be a shortage of characters available for operand con-
      straints.  Thus it may be time to change the current single letter to
      two or three letter combinations (like Epiphany, SH, etc.), except for
      "abAIJKL" which are already listed in the user manual and cannot be
      changed for compatibility reasons.

(ii.) It seems that there is no processing for PRE/POST_MODIFY side-effects
       whose displacement is a register, i.e., the RTX
         "(pre/post_modify:P (reg:P X) (plus:P (reg:P X) (reg:P Y)))".
       Of course, that is assuming that care is taken for
       TARGET_LEGITIMATE_ADDRESS_P etc.
       This target has memory reference/store machine instructions that have
       such side-effects (LSXU/SSXU or LSXP/SSXP), so we would like to
       implement them.
---
  gcc/config/xtensa/constraints.md  | 12 +++++-
  gcc/config/xtensa/xtensa-protos.h |  1 +
  gcc/config/xtensa/xtensa.cc       | 70 +++++++++++++++++++++++++++++--
  gcc/config/xtensa/xtensa.h        |  7 ++++
  gcc/config/xtensa/xtensa.md       | 66 +----------------------------
  5 files changed, 87 insertions(+), 69 deletions(-)
  

Comments

Max Filippov Sept. 16, 2025, 10:52 a.m. UTC | #1
Hi Suwa-san,

On Sat, Sep 13, 2025 at 3:43 AM Takayuki 'January June' Suwa
<jjsuwa_sys3175@yahoo.co.jp> wrote:
>
> The Xtensa ISA has hardware FP register load/store instructions that have
> pre- or post-modify side effects (LSIU/SSIU or LSIP/SSIP machine instruc-
> tions) depending on the configuration and currently these are defined using
> parallel RTX, but in fact they do not work at all:
>
>    "An instruction that can be represented with an embedded side effect
>     could also be represented using parallel containing an additional set
>     to describe how the address register is altered.  This is not done
>     because machines that allow these operations at all typically allow
>     them wherever a memory address is called for.  Describing them as
>     additional parallel stores would require doubling the number of entries
>     in the machine description."
>
>    -- 13.16 Embedded Side-Effects on Addresses, gccint (the latest)
>
> Looking back for the commit history, this has been the case since gcc 3,
> and there was a similar description in gccint at the time, so there must
> have been some reason that we cannot think of at the moment.
>
> This patch re-implements support for the above instructions in a modern
> manner, in other words, using appropriate embedded side-effect RTXes.
>
> gcc/ChangeLog:
>
>         * config/xtensa/constraints.md (U): Add condition that the address
>         must have no side-effects.
>         (S): New constraint to represent non-memory pool reference whose
>         address has some side-effect.
>         * config/xtensa/xtensa.h
>         (HAVE_PRE_MODIFY_DISP, HAVE_POST_MODIFY_DISP): New macros.
>         * config/xtensa/xtensa-protos.h (sideeffect_addr_mem_p):
>         New function prototype.
>         * config/xtensa/xtensa.cc (sideeffect_addr_mem_p):
>         New function definition that returns true if the address of the
>         specified mem RTX has a supported side-effect.
>         (TARGET_LEGITIMATE_ADDRESS_P): Replace a tab with a whitespace
>         (cosmetics).
>         (xtensa_legitimate_address_p): Change to return true even if the
>         address has a supported side-effect.
>         (print_operand): Add a format to output a single character to be
>         appended to the assembler mnemonic corresponding to the side-effect
>         of the specified memory operand.
>         (print_operand_address): Change to accept addresses even if they
>         have side-effects.
>         (pprint_operand, xtensa_output_literal): Replace fputs() of
>         a string consisting of a single character with fputc() of that
>         character.
>         * config/xtensa/xtensa.md (*lsiu, *ssiu, *lsip, *ssip):
>         Remove due to breakdown.
>         (movsf_internal): Add templates for memory reference/store machine
>         instructions with pre/post_modify properties.
>
> --- >8 ---
>
> (Notes to be worried about...)
>
> There are two concerns I had while making this patch; one is for the
> maintainers of this target, and the other is for the GCC developer gurus
> - especially the maintainers of the RTL auto-inc-dec pass.
>
> (i.) There might be a shortage of characters available for operand con-
>       straints.  Thus it may be time to change the current single letter to
>       two or three letter combinations (like Epiphany, SH, etc.), except for
>       "abAIJKL" which are already listed in the user manual and cannot be
>       changed for compatibility reasons.
>
> (ii.) It seems that there is no processing for PRE/POST_MODIFY side-effects
>        whose displacement is a register, i.e., the RTX
>          "(pre/post_modify:P (reg:P X) (plus:P (reg:P X) (reg:P Y)))".
>        Of course, that is assuming that care is taken for
>        TARGET_LEGITIMATE_ADDRESS_P etc.
>        This target has memory reference/store machine instructions that have
>        such side-effects (LSXU/SSXU or LSXP/SSXP), so we would like to
>        implement them.
> ---
>   gcc/config/xtensa/constraints.md  | 12 +++++-
>   gcc/config/xtensa/xtensa-protos.h |  1 +
>   gcc/config/xtensa/xtensa.cc       | 70 +++++++++++++++++++++++++++++--
>   gcc/config/xtensa/xtensa.h        |  7 ++++
>   gcc/config/xtensa/xtensa.md       | 66 +----------------------------
>   5 files changed, 87 insertions(+), 69 deletions(-)

This change introduces 531 new regressions in execution tests on a configuration
with FPU. Some failures are not related to the FP code, e.g.:

gcc.c-torture/execute/990524-1.c   -O1  execution test
  

Patch

diff --git a/gcc/config/xtensa/constraints.md b/gcc/config/xtensa/constraints.md
index 727ec1e2c20..1cf697e1797 100644
--- a/gcc/config/xtensa/constraints.md
+++ b/gcc/config/xtensa/constraints.md
@@ -136,6 +136,14 @@ 
        (match_test "!TARGET_CONST16 && constantpool_mem_p (op)")))
  
  (define_memory_constraint "U"
- "Memory that is not in a literal pool."
+ "Memory that is not in a literal pool, and its address has no side-effect."
   (and (match_code "mem")
-      (match_test "! constantpool_mem_p (op)")))
+      (match_test "! constantpool_mem_p (op)
+		   && ! sideeffect_addr_mem_p (op)")))
+
+(define_memory_constraint "S"
+ "Memory that is not in a literal pool, and its address has a side-effect
+  (such as pre/post-modify)."
+ (and (match_code "mem")
+      (match_test "! constantpool_mem_p (op)
+		   && sideeffect_addr_mem_p (op)")))
diff --git a/gcc/config/xtensa/xtensa-protos.h b/gcc/config/xtensa/xtensa-protos.h
index 98e75c6a535..688bca172ac 100644
--- a/gcc/config/xtensa/xtensa-protos.h
+++ b/gcc/config/xtensa/xtensa-protos.h
@@ -37,6 +37,7 @@  extern int xt_true_regnum (rtx);
  extern int xtensa_valid_move (machine_mode, rtx *);
  extern int smalloffset_mem_p (rtx);
  extern int constantpool_mem_p (rtx);
+extern bool sideeffect_addr_mem_p (rtx);
  extern void xtensa_extend_reg (rtx, rtx);
  extern void xtensa_expand_conditional_branch (rtx *, machine_mode);
  extern int xtensa_expand_conditional_move (rtx *, int);
diff --git a/gcc/config/xtensa/xtensa.cc b/gcc/config/xtensa/xtensa.cc
index 2e7a9df32d2..b10cf76200d 100644
--- a/gcc/config/xtensa/xtensa.cc
+++ b/gcc/config/xtensa/xtensa.cc
@@ -293,7 +293,7 @@  static HARD_REG_SET xtensa_zero_call_used_regs (HARD_REG_SET);
  #define TARGET_CANNOT_FORCE_CONST_MEM xtensa_cannot_force_const_mem
  
  #undef TARGET_LEGITIMATE_ADDRESS_P
-#define TARGET_LEGITIMATE_ADDRESS_P	xtensa_legitimate_address_p
+#define TARGET_LEGITIMATE_ADDRESS_P xtensa_legitimate_address_p
  
  #undef TARGET_FRAME_POINTER_REQUIRED
  #define TARGET_FRAME_POINTER_REQUIRED xtensa_frame_pointer_required
@@ -627,6 +627,35 @@  constantpool_mem_p (rtx op)
  }
  
  
+/* Return true if op is mem and its address has side-effect.  */
+
+bool
+sideeffect_addr_mem_p (rtx op)
+{
+  rtx dest, src;
+
+  if (SUBREG_P (op))
+    op = SUBREG_REG (op);
+  if (MEM_P (op))
+    switch (GET_CODE (op = XEXP (op, 0)))
+      {
+      case PRE_MODIFY:
+      case POST_MODIFY:
+	/* Currently, only PRE/POST_MODIFY with reg-imm addition are
+	   accepted, but not with reg-reg addition.  */
+	dest = XEXP (op, 0), src = XEXP (op, 1);
+	return (REG_P (dest)
+		&& GET_CODE (src) == PLUS
+		&& rtx_equal_p (dest, XEXP (src, 0))
+		&& CONST_INT_P (XEXP (src, 1))
+		&& xtensa_mem_offset (INTVAL (XEXP (src, 1)), SFmode));
+      default:
+	break;
+      }
+  return false;
+}
+
+
  /* Return TRUE if X is a thread-local symbol.  */
  
  static bool
@@ -2396,6 +2425,16 @@  xtensa_legitimate_address_p (machine_mode mode, rtx addr, bool strict,
    if (REG_P (addr) && BASE_REG_P (addr, strict))
      return true;
  
+  /* Check pre/post-modify addressing.  */
+  if ((GET_CODE (addr) == PRE_MODIFY
+       || GET_CODE (addr) == POST_MODIFY)
+      && REG_P (XEXP (addr, 0))
+      && GET_CODE (XEXP (addr, 1)) == PLUS
+      && rtx_equal_p (XEXP (addr, 0), XEXP (XEXP (addr, 1), 0)))
+    /* Currently, only PRE/POST_MODIFY with reg-imm addition are accepted,
+       but not with reg-reg addition.  */
+    addr = XEXP (addr, 1);
+
    /* Check for "register + offset" addressing.  */
    if (GET_CODE (addr) == PLUS)
      {
@@ -3163,6 +3202,8 @@  xtensa_modes_tieable_p (machine_mode mode1, machine_mode mode2)
     'D'  REG, print second register of double-word register operand
     'N'  MEM, print address of next word following a memory operand
     'v'  MEM, if memory reference is volatile, output a MEMW before it
+   's'  MEM, if memory reference has side-effect, output a character
+	corresponding to
     't'  any constant, add "@h" suffix for top 16 bits
     'b'  any constant, add "@l" suffix for bottom 16 bits
  */
@@ -3217,6 +3258,24 @@  print_operand (FILE *file, rtx x, int letter)
  	output_operand_lossage ("invalid %%v value");
        break;
  
+    case 's':
+      if (MEM_P (x))
+	switch (GET_CODE (XEXP (x, 0)))
+	  {
+	  case PRE_MODIFY:
+	    fputc ('u', file);
+	    break;
+	  case POST_MODIFY:
+	    fputc ('p', file);
+	    break;
+	  default:
+	    output_operand_lossage ("invalid %%s value");
+	    break;
+	  }
+      else
+	output_operand_lossage ("invalid %%s value");
+      break;
+
      case 'N':
        if (MEM_P (x)
  	  && (GET_MODE (x) == DFmode || GET_MODE (x) == DImode))
@@ -3316,7 +3375,7 @@  print_operand (FILE *file, rtx x, int letter)
  	  /* There must be a non-alphanumeric character between 'h' or 'l'
  	     and the number.  The '-' is added by print_operand() already.  */
  	  if (INTVAL (XEXP (XEXP (x, 0), 1)) >= 0)
-	    fputs ("+", file);
+	    fputc ('+', file);
  	  print_operand (file, XEXP (XEXP (x, 0), 1), 0);
  	}
        else
@@ -3370,6 +3429,11 @@  print_operand_address (FILE *file, rtx addr)
        fprintf (file, "%s, 0", reg_names [REGNO (addr)]);
        break;
  
+    case PRE_MODIFY:
+    case POST_MODIFY:
+      addr = XEXP (addr, 1);
+      /* FALLTHRU */
+
      case PLUS:
        {
  	rtx reg = (rtx)0;
@@ -3506,7 +3570,7 @@  xtensa_output_literal (FILE *file, rtx x, machine_mode mode, int labelno)
      case MODE_INT:
      case MODE_PARTIAL_INT:
        xtensa_output_integer_literal_parts (file, x, GET_MODE_SIZE (mode));
-      fputs ("\n", file);
+      fputc ('\n', file);
        break;
  
      default:
diff --git a/gcc/config/xtensa/xtensa.h b/gcc/config/xtensa/xtensa.h
index 19d151366b2..06b1f71318f 100644
--- a/gcc/config/xtensa/xtensa.h
+++ b/gcc/config/xtensa/xtensa.h
@@ -602,6 +602,13 @@  typedef struct xtensa_args
    ((!(STRICT) && ! HARD_REGISTER_P (X))					\
     || REGNO_OK_FOR_BASE_P (REGNO (X)))
  
+/* C expressions that are nonzero if the machine supports pre/post-
+   address side-effect generation involving an immediate displacement.  */
+#define HAVE_PRE_MODIFY_DISP						\
+  (TARGET_HARD_FLOAT && !TARGET_HARD_FLOAT_POSTINC)
+#define HAVE_POST_MODIFY_DISP						\
+  (TARGET_HARD_FLOAT && TARGET_HARD_FLOAT_POSTINC)
+
  /* Maximum number of registers that can appear in a valid memory address.  */
  #define MAX_REGS_PER_ADDRESS 1
  
diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
index 52ffb161c0f..bc5e0587ab6 100644
--- a/gcc/config/xtensa/xtensa.md
+++ b/gcc/config/xtensa/xtensa.md
@@ -1470,6 +1470,8 @@ 
       [f,  f; farith, 3] mov.s\t%0, %1
       [f, ^U; fload , 3] %v1lsi\t%0, %1
       [U,  f; fstore, 3] %v0ssi\t%1, %0
+     [f,  S; fload , 3] %v1lsi%s1\t%0, %1
+     [S,  f; fstore, 3] %v0ssi%s0\t%1, %0
       [D,  d; move  , 2] mov.n\t%0, %1
       [a,  T; load  , 3] %v1l32r\t%0, %1
       [D,  R; load  , 2] %v1l32i.n\t%0, %1
@@ -1484,70 +1486,6 @@ 
    }
    [(set_attr "mode" "SF")])
  
-(define_insn "*lsiu"
-  [(set (match_operand:SF 0 "register_operand" "=f")
-	(mem:SF (plus:SI (match_operand:SI 1 "register_operand" "+a")
-			 (match_operand:SI 2 "fpmem_offset_operand" "i"))))
-   (set (match_dup 1)
-	(plus:SI (match_dup 1) (match_dup 2)))]
-  "TARGET_HARD_FLOAT && !TARGET_HARD_FLOAT_POSTINC"
-{
-  if (TARGET_SERIALIZE_VOLATILE && volatile_refs_p (PATTERN (insn)))
-    output_asm_insn ("memw", operands);
-  return "lsiu\t%0, %1, %2";
-}
-  [(set_attr "type"	"fload")
-   (set_attr "mode"	"SF")
-   (set_attr "length"	"3")])
-
-(define_insn "*ssiu"
-  [(set (mem:SF (plus:SI (match_operand:SI 0 "register_operand" "+a")
-			 (match_operand:SI 1 "fpmem_offset_operand" "i")))
-	(match_operand:SF 2 "register_operand" "f"))
-   (set (match_dup 0)
-	(plus:SI (match_dup 0) (match_dup 1)))]
-  "TARGET_HARD_FLOAT && !TARGET_HARD_FLOAT_POSTINC"
-{
-  if (TARGET_SERIALIZE_VOLATILE && volatile_refs_p (PATTERN (insn)))
-    output_asm_insn ("memw", operands);
-  return "ssiu\t%2, %0, %1";
-}
-  [(set_attr "type"	"fstore")
-   (set_attr "mode"	"SF")
-   (set_attr "length"	"3")])
-
-(define_insn "*lsip"
-  [(set (match_operand:SF 0 "register_operand" "=f")
-	(mem:SF (match_operand:SI 1 "register_operand" "+a")))
-   (set (match_dup 1)
-	(plus:SI (match_dup 1)
-		 (match_operand:SI 2 "fpmem_offset_operand" "i")))]
-  "TARGET_HARD_FLOAT && TARGET_HARD_FLOAT_POSTINC"
-{
-  if (TARGET_SERIALIZE_VOLATILE && volatile_refs_p (PATTERN (insn)))
-    output_asm_insn ("memw", operands);
-  return "lsip\t%0, %1, %2";
-}
-  [(set_attr "type"	"fload")
-   (set_attr "mode"	"SF")
-   (set_attr "length"	"3")])
-
-(define_insn "*ssip"
-  [(set (mem:SF (match_operand:SI 0 "register_operand" "+a"))
-	(match_operand:SF 1 "register_operand" "f"))
-   (set (match_dup 0)
-	(plus:SI (match_dup 0)
-		 (match_operand:SI 2 "fpmem_offset_operand" "i")))]
-  "TARGET_HARD_FLOAT && TARGET_HARD_FLOAT_POSTINC"
-{
-  if (TARGET_SERIALIZE_VOLATILE && volatile_refs_p (PATTERN (insn)))
-    output_asm_insn ("memw", operands);
-  return "ssip\t%1, %0, %2";
-}
-  [(set_attr "type"	"fstore")
-   (set_attr "mode"	"SF")
-   (set_attr "length"	"3")])
-
  (define_split
    [(set (match_operand:SF 0 "register_operand")
  	(match_operand 1 "constantpool_operand"))]