hppa: Improve hppa_rtx_costs for shifts by constants.

Message ID 007701d67c91$0e0eee50$2a2ccaf0$@nextmovesoftware.com
State New
Headers show
Series
  • hppa: Improve hppa_rtx_costs for shifts by constants.
Related show

Commit Message

Roger Sayle Aug. 27, 2020, 4:42 p.m.
Hi Dave (and Jeff),
For your consideration, here's a patch that should fix the recent regression
of gcc.dg/tree-ssa/slrt-13.c on hppa targets.

This patch provides more accurate rtx_costs estimates for shifts by
integer constants (which are cheaper than by a register amount).
Fine tuning these is sufficient to have simple-ssa-strength-reduce
prefer multiplications by four, over multiplications by five.

Technically only the ASHIFT is required to fix the regression, but for
symmetry
I've added the appropriate PA-RISC values for all three shift types (in
SImode).
I've also checked that this doesn't reintroduce PR middle-end/87256.

I was wondering whether you could please "put this in the queue", and
reconfirm that PR middle-end/87256 remains resolved?


2020-08-27  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	* config/pa/pa.c (hppa_rtx_costs) [ASHIFT, ASHIFTRT, LSHIFTRT]:
	Provide accurate costs for shifts of integer constants.

Many thanks in advance (and my apologies for the inconvenience),
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK

Comments

Martin Sebor via Gcc-patches Aug. 27, 2020, 6:15 p.m. | #1
On Thu, 2020-08-27 at 17:42 +0100, Roger Sayle wrote:
> Hi Dave (and Jeff),

> For your consideration, here's a patch that should fix the recent regression

> of gcc.dg/tree-ssa/slrt-13.c on hppa targets.

> 

> This patch provides more accurate rtx_costs estimates for shifts by

> integer constants (which are cheaper than by a register amount).

> Fine tuning these is sufficient to have simple-ssa-strength-reduce

> prefer multiplications by four, over multiplications by five.

> 

> Technically only the ASHIFT is required to fix the regression, but for

> symmetry

> I've added the appropriate PA-RISC values for all three shift types (in

> SImode).

> I've also checked that this doesn't reintroduce PR middle-end/87256.

> 

> I was wondering whether you could please "put this in the queue", and

> reconfirm that PR middle-end/87256 remains resolved?

> 

> 

> 2020-08-27  Roger Sayle  <roger@nextmovesoftware.com>

> 

> gcc/ChangeLog

> 	* config/pa/pa.c (hppa_rtx_costs) [ASHIFT, ASHIFTRT, LSHIFTRT]:

> 	Provide accurate costs for shifts of integer constants.

Spinning in the tester.  Figure results about this time tomorrow:

http://3.14.90.209:8080/job/hppa-linux-gnu/944/console
> 



jeff
John David Anglin Aug. 27, 2020, 6:58 p.m. | #2
Hi Roger,

On 2020-08-27 12:42 p.m., Roger Sayle wrote:
> I was wondering whether you could please "put this in the queue", and

> reconfirm that PR middle-end/87256 remains resolved?

>

>

> 2020-08-27  Roger Sayle  <roger@nextmovesoftware.com>

>

> gcc/ChangeLog

> 	* config/pa/pa.c (hppa_rtx_costs) [ASHIFT, ASHIFTRT, LSHIFTRT]:

> 	Provide accurate costs for shifts of integer constants.

Will test.

-- 
John David Anglin  dave.anglin@bell.net
John David Anglin Sept. 1, 2020, 11:53 p.m. | #3
Hi Roger,

This change is fine.  PR middle-end/87256 remains resolved.

I'll have to tweak the shadd-2 test again.  The number of sh*add instructions dropped
to three but the total number of instruction dropped by one.

Thanks,
Dave

On 2020-08-27 12:42 p.m., Roger Sayle wrote:
> Hi Dave (and Jeff),

> For your consideration, here's a patch that should fix the recent regression

> of gcc.dg/tree-ssa/slrt-13.c on hppa targets.

>

> This patch provides more accurate rtx_costs estimates for shifts by

> integer constants (which are cheaper than by a register amount).

> Fine tuning these is sufficient to have simple-ssa-strength-reduce

> prefer multiplications by four, over multiplications by five.

>

> Technically only the ASHIFT is required to fix the regression, but for

> symmetry

> I've added the appropriate PA-RISC values for all three shift types (in

> SImode).

> I've also checked that this doesn't reintroduce PR middle-end/87256.

>

> I was wondering whether you could please "put this in the queue", and

> reconfirm that PR middle-end/87256 remains resolved?

>

>

> 2020-08-27  Roger Sayle  <roger@nextmovesoftware.com>

>

> gcc/ChangeLog

> 	* config/pa/pa.c (hppa_rtx_costs) [ASHIFT, ASHIFTRT, LSHIFTRT]:

> 	Provide accurate costs for shifts of integer constants.

>

> Many thanks in advance (and my apologies for the inconvenience),

> Roger

> --

> Roger Sayle

> NextMove Software

> Cambridge, UK

>



-- 
John David Anglin  dave.anglin@bell.net

Patch

diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c
index cb88852..a9223ab 100644
--- a/gcc/config/pa/pa.c
+++ b/gcc/config/pa/pa.c
@@ -1642,6 +1642,14 @@  hppa_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	  else
 	    *total = COSTS_N_INSNS (18);
 	}
+      else if (REG_P (XEXP (x, 0)) && CONST_INT_P (XEXP (x, 1)))
+	{
+	  if (TARGET_64BIT)
+	    *total = COSTS_N_INSNS (2);
+	  else
+	    *total = COSTS_N_INSNS (1);
+	  return true;
+	}
       else if (TARGET_64BIT)
 	*total = COSTS_N_INSNS (4);
       else
@@ -1665,6 +1673,14 @@  hppa_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	  else
 	    *total = COSTS_N_INSNS (19);
 	}
+      else if (REG_P (XEXP (x, 0)) && CONST_INT_P (XEXP (x, 1)))
+	{
+	  if (TARGET_64BIT)
+	    *total = COSTS_N_INSNS (2);
+	  else
+	    *total = COSTS_N_INSNS (1);
+	  return true;
+	}
       else if (TARGET_64BIT)
 	*total = COSTS_N_INSNS (4);
       else
@@ -1688,6 +1704,11 @@  hppa_rtx_costs (rtx x, machine_mode mode, int outer_code,
 	  else
 	    *total = COSTS_N_INSNS (15);
 	}
+      else if (REG_P (XEXP (x, 0)) && CONST_INT_P (XEXP (x, 1)))
+	{
+	  *total = COSTS_N_INSNS (1);
+	  return true;
+	}
       else if (TARGET_64BIT)
 	*total = COSTS_N_INSNS (3);
       else