[PATCH] S/390: Allow to use r1 to r4 as literal pool base.

[PATCH] S/390: Allow to use r1 to r4 as literal pool base.

[Dominik Vogt]

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

The attached patch enables using r1 to r4 as the literal pool base pointer if
one of them is unused in a leaf function.  The unpatched code supports only r5
and r13.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-ChangeLog --]
[-- Type: text/plain, Size: 132 bytes --]

gcc/ChangeLog

	* config/s390/s390.c (s390_init_frame_layout): Allow to use r1, ..., r4
	as base register for the literal pool too.

[-- Attachment #3: 0001-S-390-Allow-to-use-r1-to-r4-as-literal-pool-base-poi.patch --]
[-- Type: text/x-diff, Size: 1203 bytes --]

From f2838924c4c02eeabfdf7d8b79bd7a7ce8228e06 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Fri, 11 Dec 2015 11:33:23 +0100
Subject: [PATCH] S/390: Allow to use r1 to r4 as literal pool base
 pointer.

The old code only considered r5 and r13.
---
 gcc/config/s390/s390.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index bc6f05b..b195ad3d 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -9543,10 +9543,17 @@ s390_init_frame_layout (void)
 	 as base register to avoid save/restore overhead.  */
       if (!base_used)
 	cfun->machine->base_reg = NULL_RTX;
-      else if (crtl->is_leaf && !df_regs_ever_live_p (5))
-	cfun->machine->base_reg = gen_rtx_REG (Pmode, 5);
       else
-	cfun->machine->base_reg = gen_rtx_REG (Pmode, BASE_REGNUM);
+	{
+	  int br = 0;
+
+	  if (crtl->is_leaf)
+	    /* Prefer r5 (most likely to be free).  */
+	    for (br = 5; br >= 1 && df_regs_ever_live_p (br); br--)
+	      ;
+	  cfun->machine->base_reg =
+	    gen_rtx_REG (Pmode, (br > 0) ? br : BASE_REGNUM);
+	}
 
       s390_register_info ();
       s390_frame_info ();
-- 
2.3.0

Re: [PATCH] S/390: Allow to use r1 to r4 as literal pool base.

[Ulrich Weigand]

Dominik Vogt wrote:

> The attached patch enables using r1 to r4 as the literal pool base pointer if
> one of them is unused in a leaf function.  The unpatched code supports only r5
> and r13.

I don't think that r1 is actually safe here.  Note that it may be used
(unconditionally) as temp register in s390_emit_prologue in certain cases;
the upcoming split-stack code will also need to use r1 in some cases.

r2 through r4 should be fine.  [ Not sure if there will be many (any?) cases
where one of those is unused but r5 isn't, however. ]

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] S/390: Allow to use r1 to r4 as literal pool base.

[Dominik Vogt]

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

On Mon, Dec 14, 2015 at 04:08:32PM +0100, Ulrich Weigand wrote:
> Dominik Vogt wrote:
> 
> > The attached patch enables using r1 to r4 as the literal pool base pointer if
> > one of them is unused in a leaf function.  The unpatched code supports only r5
> > and r13.
> 
> I don't think that r1 is actually safe here.  Note that it may be used
> (unconditionally) as temp register in s390_emit_prologue in certain cases;
> the upcoming split-stack code will also need to use r1 in some cases.

How about the attached patch?  It also allows to use r0 as the
temp register if possible (needs more testing).  If that's too
much effort, I'm fine with limiting the original patch to r4 to
r2.

> r2 through r4 should be fine.  [ Not sure if there will be many (any?) cases
> where one of those is unused but r5 isn't, however. ]

This can happen if the function only uses register pairs
(__int128).  Actually I'm not sure whether r2 and r4 are valid
candidates.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-ChangeLog --]
[-- Type: text/plain, Size: 325 bytes --]

gcc/ChangeLog

	* config/s390/s390.c (s390_init_frame_layout): Try r4 to r1 for the
	literal pool pointer.
	(s390_get_prologue_temp_regno): New function to choose the temp_reg for
	the prologue.  Allow to use r0 if that's safe.
	(s390_emit_prologue): Move code choosing the temp_reg to a separate
	function.  Add assertions.

[-- Attachment #3: 0001-S-390-Allow-to-use-r1-to-r4-as-literal-pool-base-poi.patch --]
[-- Type: text/x-diff, Size: 3772 bytes --]

From 806973409adc48c8ca701d55fdbad897b0e31c78 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Fri, 11 Dec 2015 11:33:23 +0100
Subject: [PATCH] S/390: Allow to use r1 to r4 as literal pool base
 pointer.

The old code only considered r5 and r13.
---
 gcc/config/s390/s390.c | 61 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index bc6f05b..c45b992 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -9506,6 +9506,26 @@ s390_frame_info (void)
 				  & ~(STACK_BOUNDARY / BITS_PER_UNIT - 1));
 }
 
+/* Returns the register number that is used as a temp register in the prologue.
+ */
+static int
+s390_get_prologue_temp_regno (void)
+{
+  if (!has_hard_reg_initial_val (Pmode, RETURN_REGNUM)
+      && !crtl->is_leaf
+      && !TARGET_TPF_PROFILING)
+    return RETURN_REGNUM;
+  if (cfun_save_high_fprs_p)
+    /* Needs an address register.  */
+    return 1;
+  else if (TARGET_BACKCHAIN)
+    /* Does not need an address register.  */
+    return 0;
+
+  /* No temp register needed.  */
+  return -1;
+}
+
 /* Generate frame layout.  Fills in register and frame data for the current
    function in cfun->machine.  This routine can be called multiple times;
    it will re-do the complete frame layout every time.  */
@@ -9543,10 +9563,24 @@ s390_init_frame_layout (void)
 	 as base register to avoid save/restore overhead.  */
       if (!base_used)
 	cfun->machine->base_reg = NULL_RTX;
-      else if (crtl->is_leaf && !df_regs_ever_live_p (5))
-	cfun->machine->base_reg = gen_rtx_REG (Pmode, 5);
       else
-	cfun->machine->base_reg = gen_rtx_REG (Pmode, BASE_REGNUM);
+	{
+	  int br = 0;
+
+	  if (crtl->is_leaf)
+	    {
+	      int temp_regno;
+
+	      temp_regno = s390_get_prologue_temp_regno ();
+	      /* Prefer r5 (most likely to be free).  */
+	      for (br = 5;
+		   br >= 1 && (br == temp_regno || df_regs_ever_live_p (br));
+		   br--)
+		;
+	    }
+	  cfun->machine->base_reg =
+	    gen_rtx_REG (Pmode, (br > 0) ? br : BASE_REGNUM);
+	}
 
       s390_register_info ();
       s390_frame_info ();
@@ -10385,19 +10419,16 @@ s390_emit_prologue (void)
 {
   rtx insn, addr;
   rtx temp_reg;
+  int temp_regno;
   int i;
   int offset;
   int next_fpr = 0;
 
-  /* Choose best register to use for temp use within prologue.
-     See below for why TPF must use the register 1.  */
-
-  if (!has_hard_reg_initial_val (Pmode, RETURN_REGNUM)
-      && !crtl->is_leaf
-      && !TARGET_TPF_PROFILING)
-    temp_reg = gen_rtx_REG (Pmode, RETURN_REGNUM);
-  else
-    temp_reg = gen_rtx_REG (Pmode, 1);
+  /* If new uses of temp_reg are introduced into the prologue, be sure to
+     update the conditions in s390_get_prologue_temp_regno().  Otherwise the
+     prologue might overwrite the literal pool pointer in r1.  */
+  temp_regno = s390_get_prologue_temp_regno ();
+  temp_reg = (temp_regno >= 0) ? gen_rtx_REG (Pmode, temp_regno) : NULL_RTX;
 
   s390_save_gprs_to_fprs ();
 
@@ -10551,7 +10582,10 @@ s390_emit_prologue (void)
 
       /* Save incoming stack pointer into temp reg.  */
       if (TARGET_BACKCHAIN || next_fpr)
-	insn = emit_insn (gen_move_insn (temp_reg, stack_pointer_rtx));
+	{
+	  gcc_assert (temp_regno >= 0);
+	  insn = emit_insn (gen_move_insn (temp_reg, stack_pointer_rtx));
+	}
 
       /* Subtract frame size from stack pointer.  */
 
@@ -10606,6 +10640,7 @@ s390_emit_prologue (void)
 
   if (cfun_save_high_fprs_p && next_fpr)
     {
+      gcc_assert (temp_regno >= 1);
       /* If the stack might be accessed through a different register
 	 we have to make sure that the stack pointer decrement is not
 	 moved below the use of the stack slots.  */
-- 
2.3.0

Re: [PATCH] S/390: Allow to use r1 to r4 as literal pool base.

[Ulrich Weigand]

Dominik Vogt wrote:
> On Mon, Dec 14, 2015 at 04:08:32PM +0100, Ulrich Weigand wrote:
> > I don't think that r1 is actually safe here.  Note that it may be used
> > (unconditionally) as temp register in s390_emit_prologue in certain cases;
> > the upcoming split-stack code will also need to use r1 in some cases.
> 
> How about the attached patch?  It also allows to use r0 as the
> temp register if possible (needs more testing).

This doesn't look safe either.  In particular:

- you use cfun_save_high_fprs_p at a place where its value might not yet
  have been determined (when calling s390_get_prologue_temp_regno from
  s390_init_frame_layout *before* the s390_register_info/s390_frame_info
  calls)

- r0 might hold the incoming static chain value, in which case it cannot
  be used as temp register

> If that's too
> much effort, I'm fine with limiting the original patch to r4 to
> r2.

That seems preferable to me.

> > r2 through r4 should be fine.  [ Not sure if there will be many (any?) cases
> > where one of those is unused but r5 isn't, however. ]
> 
> This can happen if the function only uses register pairs
> (__int128).  Actually I'm not sure whether r2 and r4 are valid
> candidates.

Huh?  Why not?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] S/390: Allow to use r1 to r4 as literal pool base.

[Dominik Vogt]

On Wed, Dec 16, 2015 at 01:51:45PM +0100, Ulrich Weigand wrote:
> Dominik Vogt wrote:
> > > r2 through r4 should be fine.  [ Not sure if there will be many (any?) cases
> > > where one of those is unused but r5 isn't, however. ]
> > 
> > This can happen if the function only uses register pairs
> > (__int128).  Actually I'm not sure whether r2 and r4 are valid
> > candidates.
> 
> Huh?  Why not?

Because I'm not sure it is possible to write code where r2 (r4) is
free but r3 (r5) is not - at least when s390_emit_prologue is
called.  Writing code that uses r4 and r5 but not r3 was diffucult
enough:

  __int128 gi;
  const int c = 0x12345678u;
  int foo(void)
  {
    gi += c;
    return c;
  }

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] S/390: Allow to use r1 to r4 as literal pool base.

[Ulrich Weigand]

Dominik Vogt wrote:
> On Wed, Dec 16, 2015 at 01:51:45PM +0100, Ulrich Weigand wrote:
> > Dominik Vogt wrote:
> > > > r2 through r4 should be fine.  [ Not sure if there will be many (any?) cases
> > > > where one of those is unused but r5 isn't, however. ]
> > > 
> > > This can happen if the function only uses register pairs
> > > (__int128).  Actually I'm not sure whether r2 and r4 are valid
> > > candidates.
> > 
> > Huh?  Why not?
> 
> Because I'm not sure it is possible to write code where r2 (r4) is
> free but r3 (r5) is not - at least when s390_emit_prologue is
> called.  Writing code that uses r4 and r5 but not r3 was diffucult
> enough:

Ah, OK.  I agree that it will rarely happen (it could in more
complex cases where something initially uses r2 but a very late
optimization pass manages to eliminate that use).

However, when it is *is* free, it is a valid candidate in the
sense that it would be safe and correct to use it.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] S/390: Allow to use r2 to r4 as literal pool base.

[Dominik Vogt]

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

Latest patch for r2 to r4, including a test case.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-ChangeLog --]
[-- Type: text/plain, Size: 177 bytes --]

gcc/ChangeLog

	* config/s390/s390.c (s390_init_frame_layout): Try r4 to r2 for the
	literal pool pointer.
gcc/testsuite/ChangeLog

	* gcc.target/s390/litpool-r3-1.c: New test.

[-- Attachment #3: 0001-S-390-Allow-to-use-r2-to-r4-as-literal-pool-base-poi.patch --]
[-- Type: text/x-diff, Size: 2014 bytes --]

From 67073c8b769e857d709f5e613919a6fe0120202e Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Fri, 11 Dec 2015 11:33:23 +0100
Subject: [PATCH] S/390: Allow to use r2 to r4 as literal pool base
 pointer.

The old code only considered r5 and r13.
---
 gcc/config/s390/s390.c                       | 13 ++++++++++---
 gcc/testsuite/gcc.target/s390/litpool-r3-1.c | 16 ++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/litpool-r3-1.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index ed684af..cba88bb 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -9584,10 +9584,17 @@ s390_init_frame_layout (void)
 	 as base register to avoid save/restore overhead.  */
       if (!base_used)
 	cfun->machine->base_reg = NULL_RTX;
-      else if (crtl->is_leaf && !df_regs_ever_live_p (5))
-	cfun->machine->base_reg = gen_rtx_REG (Pmode, 5);
       else
-	cfun->machine->base_reg = gen_rtx_REG (Pmode, BASE_REGNUM);
+	{
+	  int br = 0;
+
+	  if (crtl->is_leaf)
+	    /* Prefer r5 (most likely to be free).  */
+	    for (br = 5; br >= 2 && df_regs_ever_live_p (br); br--)
+	      ;
+	  cfun->machine->base_reg =
+	    gen_rtx_REG (Pmode, (br > 0) ? br : BASE_REGNUM);
+	}
 
       s390_register_info ();
       s390_frame_info ();
diff --git a/gcc/testsuite/gcc.target/s390/litpool-r3-1.c b/gcc/testsuite/gcc.target/s390/litpool-r3-1.c
new file mode 100644
index 0000000..8ee50cf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/litpool-r3-1.c
@@ -0,0 +1,16 @@
+/* Validate that r3 may be used as the literal pool pointer.  Test that only on
+   64-bit for z900 to simplify the test.  It's not really different on 31-bit
+   or other cpus.  */
+
+/* { dg-do compile { target { lp64 } } } */
+/* { dg-options "-march=z900 -O2" } */
+
+__int128 gi;
+const int c = 0x12345678u;
+int foo(void)
+{
+	gi += c;
+	return c;
+}
+
+/* { dg-final { scan-assembler-times "\tlarl\t%r3,.L3" 1 } } */
-- 
2.3.0

Re: [PATCH] S/390: Allow to use r2 to r4 as literal pool base.

[Andreas Krebbel]

On 12/18/2015 11:21 AM, Dominik Vogt wrote:
> Latest patch for r2 to r4, including a test case.
gcc/ChangeLog

	* config/s390/s390.c (s390_init_frame_layout): Try r4 to r2 for the
	literal pool pointer.
gcc/testsuite/ChangeLog

	* gcc.target/s390/litpool-r3-1.c: New test.

Applied.  Thanks!

-Andreas-

Re: [PATCH] S/390: Allow to use r2 to r4 as literal pool base.

[Jakub Jelinek]

On Fri, Dec 18, 2015 at 11:21:38AM +0100, Dominik Vogt wrote:
> Latest patch for r2 to r4, including a test case.
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index ed684af..cba88bb 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -9584,10 +9584,17 @@ s390_init_frame_layout (void)
>  	 as base register to avoid save/restore overhead.  */
>        if (!base_used)
>  	cfun->machine->base_reg = NULL_RTX;
> -      else if (crtl->is_leaf && !df_regs_ever_live_p (5))
> -	cfun->machine->base_reg = gen_rtx_REG (Pmode, 5);
>        else
> -	cfun->machine->base_reg = gen_rtx_REG (Pmode, BASE_REGNUM);
> +	{
> +	  int br = 0;
> +
> +	  if (crtl->is_leaf)
> +	    /* Prefer r5 (most likely to be free).  */
> +	    for (br = 5; br >= 2 && df_regs_ever_live_p (br); br--)
> +	      ;
> +	  cfun->machine->base_reg =
> +	    gen_rtx_REG (Pmode, (br > 0) ? br : BASE_REGNUM);

Note the formatting is wrong.  It should be:
	  cfun->machine->base_reg
	    = gen_rtx_REG (Pmode, (br > 0) ? br : BASE_REGNUM);

	Jakub