public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/3] VAX backend preparatory updates for switching to LRA
@ 2021-04-21 21:32 Maciej W. Rozycki
  2021-04-21 21:33 ` [PATCH 1/3] VAX: Remove dead `adjacent_operands_p' function Maciej W. Rozycki
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2021-04-21 21:32 UTC (permalink / raw)
  To: gcc-patches

Hi,

 According to the plan discussed in the context of the recent switch to
MODE_CC of the VAX backend I have been looking into switching the backend 
to LRA as well.

 It has turned out quite straightforward itself, with just a couple of 
minor issues triggered with a flip to LRA, one causing a build failure 
with target libatomic and another causing a C testsuite regression.  Also 
I have come across a piece of dead code which has never ever been used for 
anything and it is unclear to me what its intended purpose was.

 I have come up with this small patch series then, bundled together for 
easier reference although the individual changes are independent from each 
other.

 I think 3/3 is worth backporting to GCC 11 at one point, perhaps 11.2, so 
that it can be easily picked downstream, as it improves code generation 
with old reload and we may not have another major release still using it.

 OTOH switching to LRA regresses code generation seriously, by making the 
indexed and indirect VAX address modes severely underutilised, so while 
with these changes in place the backend can be switched to LRA with just a 
trivial to remove the redefinition of TARGET_LRA_P, I think it is not yet 
the right time to do it.

 It is not a hard show-stopper though, so while I plan to look into LRA 
now to figure out what is missing there that the old reload has to satisfy 
the VAX backend, the switch to LRA can now be made anytime if so required 
and I am preempted for whatever reason (and nobody else gets to it).

 Questions, comments, OK to apply?

  Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] VAX: Remove dead `adjacent_operands_p' function
  2021-04-21 21:32 [PATCH 0/3] VAX backend preparatory updates for switching to LRA Maciej W. Rozycki
@ 2021-04-21 21:33 ` Maciej W. Rozycki
  2021-04-21 21:33 ` [PATCH 2/3] VAX: Fix ill-formed `jbb<ccss>i<mode>' insn operands Maciej W. Rozycki
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2021-04-21 21:33 UTC (permalink / raw)
  To: gcc-patches

This function has never been used and it is unclear what its intended 
purpose was.

	gcc/
	* config/vax/vax-protos.h (adjacent_operands_p): Remove 
	prototype.
	* config/vax/vax.c (adjacent_operands_p): Remove.
---
 gcc/config/vax/vax-protos.h |    1 
 gcc/config/vax/vax.c        |   73 --------------------------------------------
 2 files changed, 74 deletions(-)

gcc-vax-adjacent_operands.diff
Index: gcc/gcc/config/vax/vax-protos.h
===================================================================
--- gcc.orig/gcc/config/vax/vax-protos.h
+++ gcc/gcc/config/vax/vax-protos.h
@@ -24,7 +24,6 @@ extern void vax_expand_prologue (void);
 extern bool vax_acceptable_pic_operand_p (rtx, bool, bool);
 extern machine_mode vax_select_cc_mode (enum rtx_code, rtx, rtx);
 extern const char *cond_name (rtx);
-extern bool adjacent_operands_p (rtx, rtx, machine_mode);
 extern const char *rev_cond_name (rtx);
 extern void print_operand_address (FILE *, rtx);
 extern void print_operand (FILE *, rtx, int);
Index: gcc/gcc/config/vax/vax.c
===================================================================
--- gcc.orig/gcc/config/vax/vax.c
+++ gcc/gcc/config/vax/vax.c
@@ -2108,79 +2108,6 @@ vax_expand_addsub_di_operands (rtx * ope
     }
 }
 
-bool
-adjacent_operands_p (rtx lo, rtx hi, machine_mode mode)
-{
-  HOST_WIDE_INT lo_offset;
-  HOST_WIDE_INT hi_offset;
-
-  if (GET_CODE (lo) != GET_CODE (hi))
-    return false;
-
-  if (REG_P (lo))
-    return mode == SImode && REGNO (lo) + 1 == REGNO (hi);
-  if (CONST_INT_P (lo))
-    return INTVAL (hi) == 0 && UINTVAL (lo) < 64;
-  if (CONST_INT_P (lo))
-    return mode != SImode;
-
-  if (!MEM_P (lo))
-    return false;
-
-  if (MEM_VOLATILE_P (lo) || MEM_VOLATILE_P (hi))
-    return false;
-
-  lo = XEXP (lo, 0);
-  hi = XEXP (hi, 0);
-
-  if (GET_CODE (lo) == POST_INC /* || GET_CODE (lo) == PRE_DEC */)
-    return rtx_equal_p (lo, hi);
-
-  switch (GET_CODE (lo))
-    {
-    case REG:
-    case SYMBOL_REF:
-      lo_offset = 0;
-      break;
-    case CONST:
-      lo = XEXP (lo, 0);
-      /* FALLTHROUGH */
-    case PLUS:
-      if (!CONST_INT_P (XEXP (lo, 1)))
-	return false;
-      lo_offset = INTVAL (XEXP (lo, 1));
-      lo = XEXP (lo, 0);
-      break;
-    default:
-      return false;
-    }
-
-  switch (GET_CODE (hi))
-    {
-    case REG:
-    case SYMBOL_REF:
-      hi_offset = 0;
-      break;
-    case CONST:
-      hi = XEXP (hi, 0);
-      /* FALLTHROUGH */
-    case PLUS:
-      if (!CONST_INT_P (XEXP (hi, 1)))
-	return false;
-      hi_offset = INTVAL (XEXP (hi, 1));
-      hi = XEXP (hi, 0);
-      break;
-    default:
-      return false;
-    }
-
-  if (GET_CODE (lo) == MULT || GET_CODE (lo) == PLUS)
-    return false;
-
-  return rtx_equal_p (lo, hi)
-	 && hi_offset - lo_offset == GET_MODE_SIZE (mode);
-}
-
 /* Output assembler code for a block containing the constant parts
    of a trampoline, leaving space for the variable parts.  */
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] VAX: Fix ill-formed `jbb<ccss>i<mode>' insn operands
  2021-04-21 21:32 [PATCH 0/3] VAX backend preparatory updates for switching to LRA Maciej W. Rozycki
  2021-04-21 21:33 ` [PATCH 1/3] VAX: Remove dead `adjacent_operands_p' function Maciej W. Rozycki
@ 2021-04-21 21:33 ` Maciej W. Rozycki
  2021-04-21 21:33 ` [PATCH 3/3] VAX: Accept ASHIFT in address expressions Maciej W. Rozycki
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2021-04-21 21:33 UTC (permalink / raw)
  To: gcc-patches

The insn has extraneous operand #3 that is aliased in RTL to operand #0 
with a constraint.  The operands specify a single-bit field in memory 
that the machine instruction produced boths reads for the purpose of 
determining whether to branch or not and either clears or sets according 
to the machine operation selected with the `ccss' iterator.  The caller 
of the insn is supposed to supply the same rtx for both operands.

This odd arrangement happens to work with old reload, but breaks with 
libatomic if LRA is used instead:

.../libatomic/flag.c: In function 'atomic_flag_test_and_set':
.../libatomic/flag.c:36:1: error: unable to generate reloads for:
   36 | }
      | ^
(jump_insn 7 6 19 2 (unspec_volatile [
            (set (pc)
                (if_then_else (eq (zero_extract:SI (mem/v:QI (reg:SI 27) [-1  S1 A8])
                            (const_int 1 [0x1])
                            (const_int 0 [0]))
                        (const_int 1 [0x1]))
                    (label_ref:SI 25)
                    (pc)))
            (set (zero_extract:SI (mem/v:QI (reg:SI 28) [-1  S1 A8])
                    (const_int 1 [0x1])
                    (const_int 0 [0]))
                (const_int 1 [0x1]))
        ] 100) ".../libatomic/flag.c":35:10 669 {jbbssiqi}
     (nil)
 -> 25)
during RTL pass: reload
.../libatomic/flag.c:36:1: internal compiler error: in curr_insn_transform, at lra-constraints.c:4098
0x1112c587 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
	.../gcc/rtl-error.c:108
0x10ee6563 curr_insn_transform
	.../gcc/lra-constraints.c:4098
0x10eeaf87 lra_constraints(bool)
	.../gcc/lra-constraints.c:5133
0x10ec97e3 lra(_IO_FILE*)
	.../gcc/lra.c:2336
0x10e4633f do_reload
	.../gcc/ira.c:5827
0x10e46b27 execute
	.../gcc/ira.c:6013
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

Switch to using `match_dup' as expected then for a machine instruction 
that in its encoding only has one actual operand in for the single-bit 
field.

	gcc/
	* config/vax/builtins.md (jbb<ccss>i<mode>): Remove operand #3.
	(sync_lock_test_and_set<mode>): Adjust accordingly.
	(sync_lock_release<mode>): Likewise.
---
 gcc/config/vax/builtins.md |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

gcc-vax-jbbxxi-ops.diff
Index: gcc/gcc/config/vax/builtins.md
===================================================================
--- gcc.orig/gcc/config/vax/builtins.md
+++ gcc/gcc/config/vax/builtins.md
@@ -174,8 +174,7 @@
 
   label = gen_label_rtx ();
   emit_move_insn (operands[0], const1_rtx);
-  emit_jump_insn (gen_jbbssi<mode> (operands[1], const0_rtx, label,
-				    operands[1]));
+  emit_jump_insn (gen_jbbssi<mode> (operands[1], const0_rtx, label));
   emit_move_insn (operands[0], const0_rtx);
   emit_label (label);
   DONE;
@@ -193,8 +192,7 @@
     FAIL;
 
   label = gen_label_rtx ();
-  emit_jump_insn (gen_jbbcci<mode> (operands[0], const0_rtx, label,
-				    operands[0]));
+  emit_jump_insn (gen_jbbcci<mode> (operands[0], const0_rtx, label));
   emit_label (label);
   DONE;
 }")
@@ -204,13 +202,13 @@
     [(set (pc)
 	  (if_then_else
 	    (eq (zero_extract:SI
-		  (match_operand:VAXint 0 "any_memory_operand" "<bb_mem>")
+		  (match_operand:VAXint 0 "any_memory_operand" "+<bb_mem>")
 		  (const_int 1)
 		  (match_operand:SI 1 "general_operand" "nrmT"))
 		(const_int bit))
 	    (label_ref (match_operand 2 "" ""))
 	    (pc)))
-     (set (zero_extract:SI (match_operand:VAXint 3 "any_memory_operand" "+0")
+     (set (zero_extract:SI (match_dup 0)
 			   (const_int 1)
 			   (match_dup 1))
 	  (const_int bit))]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] VAX: Accept ASHIFT in address expressions
  2021-04-21 21:32 [PATCH 0/3] VAX backend preparatory updates for switching to LRA Maciej W. Rozycki
  2021-04-21 21:33 ` [PATCH 1/3] VAX: Remove dead `adjacent_operands_p' function Maciej W. Rozycki
  2021-04-21 21:33 ` [PATCH 2/3] VAX: Fix ill-formed `jbb<ccss>i<mode>' insn operands Maciej W. Rozycki
@ 2021-04-21 21:33 ` Maciej W. Rozycki
  2021-04-22  8:16 ` [PATCH 0/3] VAX backend preparatory updates for switching to LRA Richard Biener
  2021-04-22 13:52 ` Koning, Paul
  4 siblings, 0 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2021-04-21 21:33 UTC (permalink / raw)
  To: gcc-patches

Fix regressions:

FAIL: gcc.c-torture/execute/20090113-2.c   -O1  (internal compiler error)
FAIL: gcc.c-torture/execute/20090113-2.c   -O1  (test for excess errors)
FAIL: gcc.c-torture/execute/20090113-3.c   -O1  (internal compiler error)
FAIL: gcc.c-torture/execute/20090113-3.c   -O1  (test for excess errors)

triggering if LRA is used rather than old reload and caused by:

(plus:SI (plus:SI (mult:SI (reg:SI 30 [ _10 ])
            (const_int 4 [0x4]))
        (reg/f:SI 26 [ _6 ]))
    (const_int 12 [0xc]))

coming from:

(insn 58 57 59 10 (set (reg:SI 33 [ _13 ])
        (zero_extract:SI (mem:SI (plus:SI (plus:SI (mult:SI (reg:SI 30 [ _10 ])
                            (const_int 4 [0x4]))
                        (reg/f:SI 26 [ _6 ]))
                    (const_int 12 [0xc])) [4 _6->bits[_10]+0 S4 A32])
            (reg:QI 56)
            (reg:SI 53))) 
".../gcc/testsuite/gcc.c-torture/execute/20090113-2.c":64:12 490 {*extzv_non_const}
     (expr_list:REG_DEAD (reg:QI 56)
        (expr_list:REG_DEAD (reg:SI 53)
            (expr_list:REG_DEAD (reg:SI 30 [ _10 ])
                (expr_list:REG_DEAD (reg/f:SI 26 [ _6 ])
                    (nil))))))

being converted into:

(plus:SI (plus:SI (ashift:SI (reg:SI 30 [ _10 ])
            (const_int 2 [0x2]))
        (reg/f:SI 26 [ _6 ]))
    (const_int 12 [0xc]))

which is an rtx the VAX backend currently does not recognize as a valid 
machine address, although apparently it is only inside MEM rtx's that 
indexed addressing is supposed to be canonicalized to a MULT rather than 
ASHIFT form.  Handle the ASHIFT form too throughout the backend then.

The change appears to also improve code generation with old reload and 
code size stats are as follows, collected from 18153 executables built 
in `check-c' GCC testing:

              samples average  median
--------------------------------------
regressions        47  0.702%  0.521%
unchanged       17503  0.000%  0.000%
progressions      603 -0.920% -0.403%
--------------------------------------
total           18153 -0.029%  0.000%

with a small number of outliers (over 5% size change):

old     new     change  %change filename
----------------------------------------------------
1885    1645    -240   -12.7320 pr53505.exe
1331    1221    -110    -8.2644 pr89634.exe
1553    1473    -80     -5.1513 stdatomic-vm.exe
1413    1341    -72     -5.0955 pr45830.exe
1415    1343    -72     -5.0883 stdatomic-vm.exe
25765   24463   -1302   -5.0533 strlen-5.exe
25765   24463   -1302   -5.0533 strlen-5.exe
25765   24463   -1302   -5.0533 strlen-5.exe
1191    1131    -60     -5.0377 20050527-1.exe

(all changes on the expansion side are below 5%).

	gcc/
	* config/vax/vax.c (print_operand_address, vax_address_cost_1)
	(index_term_p): Handle ASHIFT too.
---
 gcc/config/vax/vax.c |   34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

gcc-vax-legitimate-address-ashift.diff
Index: gcc/gcc/config/vax/vax.c
===================================================================
--- gcc.orig/gcc/config/vax/vax.c
+++ gcc/gcc/config/vax/vax.c
@@ -333,12 +333,12 @@ print_operand_address (FILE * file, rtx
 
     case PLUS:
       /* There can be either two or three things added here.  One must be a
-	 REG.  One can be either a REG or a MULT of a REG and an appropriate
-	 constant, and the third can only be a constant or a MEM.
+	 REG.  One can be either a REG or a MULT/ASHIFT of a REG and an
+	 appropriate constant, and the third can only be a constant or a MEM.
 
 	 We get these two or three things and put the constant or MEM in
-	 OFFSET, the MULT or REG in IREG, and the REG in BREG.  If we have
-	 a register and can't tell yet if it is a base or index register,
+	 OFFSET, the MULT/ASHIFT or REG in IREG, and the REG in BREG.  If we
+	 have a register and can't tell yet if it is a base or index register,
 	 put it into REG1.  */
 
       reg1 = 0; ireg = 0; breg = 0; offset = 0;
@@ -355,12 +355,14 @@ print_operand_address (FILE * file, rtx
 	  offset = XEXP (addr, 1);
 	  addr = XEXP (addr, 0);
 	}
-      else if (GET_CODE (XEXP (addr, 1)) == MULT)
+      else if (GET_CODE (XEXP (addr, 1)) == MULT
+	       || GET_CODE (XEXP (addr, 1)) == ASHIFT)
 	{
 	  ireg = XEXP (addr, 1);
 	  addr = XEXP (addr, 0);
 	}
-      else if (GET_CODE (XEXP (addr, 0)) == MULT)
+      else if (GET_CODE (XEXP (addr, 0)) == MULT
+	       || GET_CODE (XEXP (addr, 0)) == ASHIFT)
 	{
 	  ireg = XEXP (addr, 0);
 	  addr = XEXP (addr, 1);
@@ -385,7 +387,7 @@ print_operand_address (FILE * file, rtx
 	  else
 	    reg1 = addr;
 	}
-      else if (GET_CODE (addr) == MULT)
+      else if (GET_CODE (addr) == MULT || GET_CODE (addr) == ASHIFT)
 	ireg = addr;
       else
 	{
@@ -416,7 +418,8 @@ print_operand_address (FILE * file, rtx
 	    }
 	  else
 	    {
-	      gcc_assert (GET_CODE (XEXP (addr, 0)) == MULT);
+	      gcc_assert (GET_CODE (XEXP (addr, 0)) == MULT
+			  || GET_CODE (XEXP (addr, 0)) == ASHIFT);
 	      gcc_assert (!ireg);
 	      ireg = XEXP (addr, 0);
 	    }
@@ -447,7 +450,8 @@ print_operand_address (FILE * file, rtx
 	    }
 	  else
 	    {
-	      gcc_assert (GET_CODE (XEXP (addr, 1)) == MULT);
+	      gcc_assert (GET_CODE (XEXP (addr, 1)) == MULT
+			  || GET_CODE (XEXP (addr, 1)) == ASHIFT);
 	      gcc_assert (!ireg);
 	      ireg = XEXP (addr, 1);
 	    }
@@ -506,7 +510,7 @@ print_operand_address (FILE * file, rtx
 
       if (ireg != 0)
 	{
-	  if (GET_CODE (ireg) == MULT)
+	  if (GET_CODE (ireg) == MULT || GET_CODE (ireg) == ASHIFT)
 	    ireg = XEXP (ireg, 0);
 	  gcc_assert (REG_P (ireg));
 	  fprintf (file, "[%s]", reg_names[REGNO (ireg)]);
@@ -707,6 +711,7 @@ vax_address_cost_1 (rtx addr)
       reg = 1;
       break;
     case MULT:
+    case ASHIFT:
       indexed = 1;	/* 2 on VAX 2 */
       break;
     case CONST_INT:
@@ -1824,23 +1829,26 @@ static bool
 index_term_p (rtx prod, machine_mode mode, bool strict)
 {
   rtx xfoo0, xfoo1;
+  bool log_p;
 
   if (GET_MODE_SIZE (mode) == 1)
     return BASE_REGISTER_P (prod, strict);
 
-  if (GET_CODE (prod) != MULT || GET_MODE_SIZE (mode) > 8)
+  if ((GET_CODE (prod) != MULT && GET_CODE (prod) != ASHIFT)
+      || GET_MODE_SIZE (mode) > 8)
     return false;
 
+  log_p = GET_CODE (prod) == ASHIFT;
   xfoo0 = XEXP (prod, 0);
   xfoo1 = XEXP (prod, 1);
 
   if (CONST_INT_P (xfoo0)
-      && INTVAL (xfoo0) == (int)GET_MODE_SIZE (mode)
+      && GET_MODE_SIZE (mode) == (log_p ? 1 << INTVAL (xfoo0) : INTVAL (xfoo0))
       && INDEX_REGISTER_P (xfoo1, strict))
     return true;
 
   if (CONST_INT_P (xfoo1)
-      && INTVAL (xfoo1) == (int)GET_MODE_SIZE (mode)
+      && GET_MODE_SIZE (mode) == (log_p ? 1 << INTVAL (xfoo1) : INTVAL (xfoo1))
       && INDEX_REGISTER_P (xfoo0, strict))
     return true;
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] VAX backend preparatory updates for switching to LRA
  2021-04-21 21:32 [PATCH 0/3] VAX backend preparatory updates for switching to LRA Maciej W. Rozycki
                   ` (2 preceding siblings ...)
  2021-04-21 21:33 ` [PATCH 3/3] VAX: Accept ASHIFT in address expressions Maciej W. Rozycki
@ 2021-04-22  8:16 ` Richard Biener
  2021-04-27 18:19   ` Maciej W. Rozycki
  2021-04-22 13:52 ` Koning, Paul
  4 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2021-04-22  8:16 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: GCC Patches

On Thu, Apr 22, 2021 at 1:22 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> Hi,
>
>  According to the plan discussed in the context of the recent switch to
> MODE_CC of the VAX backend I have been looking into switching the backend
> to LRA as well.
>
>  It has turned out quite straightforward itself, with just a couple of
> minor issues triggered with a flip to LRA, one causing a build failure
> with target libatomic and another causing a C testsuite regression.  Also
> I have come across a piece of dead code which has never ever been used for
> anything and it is unclear to me what its intended purpose was.
>
>  I have come up with this small patch series then, bundled together for
> easier reference although the individual changes are independent from each
> other.
>
>  I think 3/3 is worth backporting to GCC 11 at one point, perhaps 11.2, so
> that it can be easily picked downstream, as it improves code generation
> with old reload and we may not have another major release still using it.
>
>  OTOH switching to LRA regresses code generation seriously, by making the
> indexed and indirect VAX address modes severely underutilised, so while
> with these changes in place the backend can be switched to LRA with just a
> trivial to remove the redefinition of TARGET_LRA_P, I think it is not yet
> the right time to do it.
>
>  It is not a hard show-stopper though, so while I plan to look into LRA
> now to figure out what is missing there that the old reload has to satisfy
> the VAX backend, the switch to LRA can now be made anytime if so required
> and I am preempted for whatever reason (and nobody else gets to it).
>
>  Questions, comments, OK to apply?

Sounds like a reasonable stance to me.  The patches look all good, thus
they are OK to apply.

Thanks,
Richard.

>
>   Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] VAX backend preparatory updates for switching to LRA
  2021-04-21 21:32 [PATCH 0/3] VAX backend preparatory updates for switching to LRA Maciej W. Rozycki
                   ` (3 preceding siblings ...)
  2021-04-22  8:16 ` [PATCH 0/3] VAX backend preparatory updates for switching to LRA Richard Biener
@ 2021-04-22 13:52 ` Koning, Paul
  4 siblings, 0 replies; 9+ messages in thread
From: Koning, Paul @ 2021-04-22 13:52 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gcc-patches



> On Apr 21, 2021, at 5:32 PM, Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> 
> ...
> OTOH switching to LRA regresses code generation seriously, by making the 
> indexed and indirect VAX address modes severely underutilised, so while 
> with these changes in place the backend can be switched to LRA with just a 
> trivial to remove the redefinition of TARGET_LRA_P, I think it is not yet 
> the right time to do it.

I noticed similar issues with pdp11, which at the moment allows LRA via a -mlra switch but doesn't make it the default.  Another mode that isn't handled well (or at all) by LRA is autoincrement/autodecrement.  It would be great if all these things could be done better, that would help several targets.  (I wonder if m68k would be another; doesn't it have similar addressing modes at least on the 68040?)

	paul



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] VAX backend preparatory updates for switching to LRA
  2021-04-22  8:16 ` [PATCH 0/3] VAX backend preparatory updates for switching to LRA Richard Biener
@ 2021-04-27 18:19   ` Maciej W. Rozycki
  2021-04-28  6:58     ` Richard Biener
  0 siblings, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2021-04-27 18:19 UTC (permalink / raw)
  To: Richard Biener, Jakub Jelinek; +Cc: GCC Patches

On Thu, 22 Apr 2021, Richard Biener wrote:

> >  I think 3/3 is worth backporting to GCC 11 at one point, perhaps 11.2, so
> > that it can be easily picked downstream, as it improves code generation
> > with old reload and we may not have another major release still using it.
> >
> >  OTOH switching to LRA regresses code generation seriously, by making the
> > indexed and indirect VAX address modes severely underutilised, so while
> > with these changes in place the backend can be switched to LRA with just a
> > trivial to remove the redefinition of TARGET_LRA_P, I think it is not yet
> > the right time to do it.
> >
> >  It is not a hard show-stopper though, so while I plan to look into LRA
> > now to figure out what is missing there that the old reload has to satisfy
> > the VAX backend, the switch to LRA can now be made anytime if so required
> > and I am preempted for whatever reason (and nobody else gets to it).
> >
> >  Questions, comments, OK to apply?
> 
> Sounds like a reasonable stance to me.  The patches look all good, thus
> they are OK to apply.

 With GCC 11.1 out now I have committed these changes.  Thank you for your 
review.

 FAOD, as noted above will it be OK if I backport 3/3 to GCC 11 now, for 
inclusion with 11.2?

 While not a regression fix the change is contained in the VAX backend, 
not a mainstream one, and now it is possibly the final opportunity to have 
old reload improved for the VAX target as it's quite likely we'll switch 
to LRA and dump old reload with GCC 12, and we may not be able to get LRA 
on a par with old reload for VAX for a while yet.  Conversely, with the 
improvement in place downstream users (NetBSD) may be able to pick it 
easily enough to make a good use of it now.

 WDYT?

  Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] VAX backend preparatory updates for switching to LRA
  2021-04-27 18:19   ` Maciej W. Rozycki
@ 2021-04-28  6:58     ` Richard Biener
  2021-05-01 16:33       ` Maciej W. Rozycki
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2021-04-28  6:58 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Jakub Jelinek, GCC Patches

On Tue, Apr 27, 2021 at 8:19 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Thu, 22 Apr 2021, Richard Biener wrote:
>
> > >  I think 3/3 is worth backporting to GCC 11 at one point, perhaps 11.2, so
> > > that it can be easily picked downstream, as it improves code generation
> > > with old reload and we may not have another major release still using it.
> > >
> > >  OTOH switching to LRA regresses code generation seriously, by making the
> > > indexed and indirect VAX address modes severely underutilised, so while
> > > with these changes in place the backend can be switched to LRA with just a
> > > trivial to remove the redefinition of TARGET_LRA_P, I think it is not yet
> > > the right time to do it.
> > >
> > >  It is not a hard show-stopper though, so while I plan to look into LRA
> > > now to figure out what is missing there that the old reload has to satisfy
> > > the VAX backend, the switch to LRA can now be made anytime if so required
> > > and I am preempted for whatever reason (and nobody else gets to it).
> > >
> > >  Questions, comments, OK to apply?
> >
> > Sounds like a reasonable stance to me.  The patches look all good, thus
> > they are OK to apply.
>
>  With GCC 11.1 out now I have committed these changes.  Thank you for your
> review.
>
>  FAOD, as noted above will it be OK if I backport 3/3 to GCC 11 now, for
> inclusion with 11.2?
>
>  While not a regression fix the change is contained in the VAX backend,
> not a mainstream one, and now it is possibly the final opportunity to have
> old reload improved for the VAX target as it's quite likely we'll switch
> to LRA and dump old reload with GCC 12, and we may not be able to get LRA
> on a par with old reload for VAX for a while yet.  Conversely, with the
> improvement in place downstream users (NetBSD) may be able to pick it
> easily enough to make a good use of it now.
>
>  WDYT?

Works for me.

Richard.

>   Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] VAX backend preparatory updates for switching to LRA
  2021-04-28  6:58     ` Richard Biener
@ 2021-05-01 16:33       ` Maciej W. Rozycki
  0 siblings, 0 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2021-05-01 16:33 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches

On Wed, 28 Apr 2021, Richard Biener wrote:

> >  FAOD, as noted above will it be OK if I backport 3/3 to GCC 11 now, for
> > inclusion with 11.2?
> >
> >  While not a regression fix the change is contained in the VAX backend,
> > not a mainstream one, and now it is possibly the final opportunity to have
> > old reload improved for the VAX target as it's quite likely we'll switch
> > to LRA and dump old reload with GCC 12, and we may not be able to get LRA
> > on a par with old reload for VAX for a while yet.  Conversely, with the
> > improvement in place downstream users (NetBSD) may be able to pick it
> > easily enough to make a good use of it now.
> >
> >  WDYT?
> 
> Works for me.

 Backport committed now then, thanks for your review.

  Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-05-01 16:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 21:32 [PATCH 0/3] VAX backend preparatory updates for switching to LRA Maciej W. Rozycki
2021-04-21 21:33 ` [PATCH 1/3] VAX: Remove dead `adjacent_operands_p' function Maciej W. Rozycki
2021-04-21 21:33 ` [PATCH 2/3] VAX: Fix ill-formed `jbb<ccss>i<mode>' insn operands Maciej W. Rozycki
2021-04-21 21:33 ` [PATCH 3/3] VAX: Accept ASHIFT in address expressions Maciej W. Rozycki
2021-04-22  8:16 ` [PATCH 0/3] VAX backend preparatory updates for switching to LRA Richard Biener
2021-04-27 18:19   ` Maciej W. Rozycki
2021-04-28  6:58     ` Richard Biener
2021-05-01 16:33       ` Maciej W. Rozycki
2021-04-22 13:52 ` Koning, Paul

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