Re: [PATCH] xtensa: Make full transition to LRA

[Max Filippov <jcmvbkbc@gmail.com>]

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

Hi Suwa-san,

I've finally processed the new issues introduced by this change.

On Wed, May 10, 2023 at 2:10 AM Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Mon, May 8, 2023 at 6:38 AM Takayuki 'January June' Suwa
> <jjsuwa_sys3175@yahoo.co.jp> wrote:
> >
> > gcc/ChangeLog:
> >
> >         * config/xtensa/constraints.md (R, T, U):
> >         Change define_constraint to define_memory_constraint.
> >         * config/xtensa/xtensa.cc
> >         (xtensa_lra_p, TARGET_LRA_P): Remove.
> >         (xtensa_emit_move_sequence): Remove "if (reload_in_progress)"
> >         clause as it can no longer be true.
> >         (xtensa_output_integer_literal_parts): Consider 16-bit wide
> >         constants.
> >         (xtensa_legitimate_constant_p): Add short-circuit path for
> >         integer load instructions.
> >         * config/xtensa/xtensa.md (movsf): Use can_create_pseudo_p()
> >         rather reload_in_progress and reload_completed.
> >         * config/xtensa/xtensa.opt (mlra): Remove.
> > ---
> >  gcc/config/xtensa/constraints.md | 26 ++++++++------------------
> >  gcc/config/xtensa/xtensa.cc      | 26 +++++---------------------
> >  gcc/config/xtensa/xtensa.md      |  2 +-
> >  gcc/config/xtensa/xtensa.opt     |  4 ----
> >  4 files changed, 14 insertions(+), 44 deletions(-)
>
> That's impressive.
> This version introduces a few execution failures in the testsuite on
> little endian targets and a bunch more (but not all, some execution
> tests still pass) on big endian.
> I'm traveling this week and likely won't be able to take a deep look
> into it until 5/15.
>
> New LE failures:

All of the LE failures are related to zero-overhead loops. Dropping the
operand 2 from the doloop_end pattern fixes that (change 1).

> New BE failures:

All of the BE failures are related to loading HImode constants into
registers, which instead of

.literal .LCx value
...
l32r register, .LCx

now generates the following code:

.literal .LCx value
.literal .LCy .LCx
...
l32r register1, .LCy
l16ui register, register1, 0

I've fixed that by allowing HImode constants in the literal pool in the
'move_operand' predicate, making addresses of such constants
legitimate in the xtensa_legitimate_address_p and adding an
alternative with l32r opcode to the movhi_internal pattern (change 2).

With these additional changes there's no new regression failures
and the generated code looks mostly the same as with the reload.

--
Thanks.
-- Max

[-- Attachment #2: 0001-gcc-xtensa-drop-operand-2-from-doloop_end-pattern.patch --]
[-- Type: text/x-patch, Size: 1035 bytes --]

From 0fb9ddfd22d11579674ac4a95912d2bc5612deb7 Mon Sep 17 00:00:00 2001
From: Max Filippov <jcmvbkbc@gmail.com>
Date: Sun, 21 Jan 2024 16:14:20 -0800
Subject: [PATCH 1/2] gcc: xtensa: drop operand 2 from doloop_end pattern

gcc/ChangeLog:
	* config/xtensa/xtensa.md (doloop_end): Drop operand 2.
---
 gcc/config/xtensa/xtensa.md | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
index 7aded86e244f..a9c37da48b81 100644
--- a/gcc/config/xtensa/xtensa.md
+++ b/gcc/config/xtensa/xtensa.md
@@ -2368,14 +2368,12 @@
 	      (set (match_dup 0)
 		   (plus:SI (match_dup 0)
 			    (const_int -1)))
-	      (unspec [(const_int 0)] UNSPEC_LSETUP_END)
-	      (clobber (match_dup 2))])] ; match_scratch
+	      (unspec [(const_int 0)] UNSPEC_LSETUP_END)])]
   "TARGET_LOOPS && optimize"
 {
   /* The loop optimizer doesn't check the predicates... */
   if (GET_MODE (operands[0]) != SImode)
     FAIL;
-  operands[2] = gen_rtx_SCRATCH (SImode);
 })
 
 \f
-- 
2.39.2


[-- Attachment #3: 0002-gcc-xtensa-fix-HImode-constant-loads.patch --]
[-- Type: text/x-patch, Size: 2987 bytes --]

From e5536a47e9f1ae856c2491919933d18866511991 Mon Sep 17 00:00:00 2001
From: Max Filippov <jcmvbkbc@gmail.com>
Date: Tue, 23 Jan 2024 10:57:21 -0800
Subject: [PATCH 2/2] gcc: xtensa: fix HImode constant loads

gcc/ChangeLog:
	* config/xtensa/predicates.md (move_operand): Don't check that a
	constant pool operand size is a multiple of UNITS_PER_WORD.
	* config/xtensa/xtensa.cc (xtensa_legitimate_address_p): Don't
	check that mode size is at least UNITS_PER_WORD.
	* config/xtensa/xtensa.md (movhi_internal): Add alternative
	loading constant from a literal pool.
---
 gcc/config/xtensa/predicates.md | 4 +---
 gcc/config/xtensa/xtensa.cc     | 2 +-
 gcc/config/xtensa/xtensa.md     | 9 +++++----
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/gcc/config/xtensa/predicates.md b/gcc/config/xtensa/predicates.md
index 672fb003a6c5..dd77911e3b70 100644
--- a/gcc/config/xtensa/predicates.md
+++ b/gcc/config/xtensa/predicates.md
@@ -143,9 +143,7 @@
 (define_predicate "move_operand"
   (ior
      (ior (match_operand 0 "register_operand")
-	  (and (match_operand 0 "memory_operand")
-	       (match_test "!constantpool_mem_p (op)
-			    || GET_MODE_SIZE (mode) % UNITS_PER_WORD == 0")))
+	  (match_operand 0 "memory_operand"))
      (ior (and (match_code "const_int")
 	       (match_test "(GET_MODE_CLASS (mode) == MODE_INT
 			     && xtensa_simm12b (INTVAL (op)))
diff --git a/gcc/config/xtensa/xtensa.cc b/gcc/config/xtensa/xtensa.cc
index 39ef576ac4a9..3c2d21fe8e2e 100644
--- a/gcc/config/xtensa/xtensa.cc
+++ b/gcc/config/xtensa/xtensa.cc
@@ -2343,7 +2343,7 @@ xtensa_legitimate_address_p (machine_mode mode, rtx addr, bool strict,
 			     code_helper)
 {
   /* Allow constant pool addresses.  */
-  if (mode != BLKmode && GET_MODE_SIZE (mode) >= UNITS_PER_WORD
+  if (mode != BLKmode
       && ! TARGET_CONST16 && constantpool_address_p (addr)
       && ! xtensa_tls_referenced_p (addr))
     return true;
diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
index a9c37da48b81..184b44e45f49 100644
--- a/gcc/config/xtensa/xtensa.md
+++ b/gcc/config/xtensa/xtensa.md
@@ -1328,8 +1328,8 @@
 })
 
 (define_insn "movhi_internal"
-  [(set (match_operand:HI 0 "nonimmed_operand" "=D,D,a,a,a,a,U,*a,*A")
-	(match_operand:HI 1 "move_operand" "M,d,r,I,Y,U,r,*A,*r"))]
+  [(set (match_operand:HI 0 "nonimmed_operand" "=D,D,a,a,a,a,a,U,*a,*A")
+	(match_operand:HI 1 "move_operand" "M,d,r,I,Y,T,U,r,*A,*r"))]
   "xtensa_valid_move (HImode, operands)"
   "@
    movi.n\t%0, %x1
@@ -1337,13 +1337,14 @@
    mov\t%0, %1
    movi\t%0, %x1
    movi\t%0, %1
+   %v1l32r\t%0, %1
    %v1l16ui\t%0, %1
    %v0s16i\t%1, %0
    rsr\t%0, ACCLO
    wsr\t%1, ACCLO"
-  [(set_attr "type"	"move,move,move,move,move,load,store,rsr,wsr")
+  [(set_attr "type"	"move,move,move,move,move,load,load,store,rsr,wsr")
    (set_attr "mode"	"HI")
-   (set_attr "length"	"2,2,3,3,3,3,3,3,3")])
+   (set_attr "length"	"2,2,3,3,3,3,3,3,3,3")])
 
 ;; 8-bit Integer moves
 
-- 
2.39.2