[committed] hppa: Revise REG+D address support to allow long displacements before reload

[committed] hppa: Revise REG+D address support to allow long displacements before reload

[John David Anglin]

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

Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.  Committed
to trunk.

This patch works around problem compiling python3.11 by improving
REG+D address handling.  The change results in smaller code and
reduced register pressure.

Dave
---

hppa: Revise REG+D address support to allow long displacements before reload

In analyzing PR rtl-optimization/112415, I realized that restricting
REG+D offsets to 5-bits before reload results in very poor code and
complexities in optimizing these instructions after reload.  The
general problem is long displacements are not allowed for floating
point accesses when generating PA 1.1 code.  Even with PA 2.0, there
is a ELF linker bug that prevents using long displacements for
floating point loads and stores.

In the past, enabling long displacements before reload caused issues
in reload.  However, there have been fixes in the handling of reloads
for floating-point accesses.  This change allows long displacements
before reload and corrects a couple of issues in the constraint
handling for integer and floating-point accesses.

2023-11-16  John David Anglin  <danglin@gcc.gnu.org>

gcc/ChangeLog:

	PR rtl-optimization/112415
	* config/pa/pa.cc (pa_legitimate_address_p): Allow 14-bit
	displacements before reload.  Simplify logic flow.  Revise
	comments.
	* config/pa/pa.h (TARGET_ELF64): New define.
	(INT14_OK_STRICT): Update define and comment.
	* config/pa/pa64-linux.h (TARGET_ELF64): Define.
	* config/pa/predicates.md (base14_operand): Don't check
	alignment of short displacements.
	(integer_store_memory_operand): Don't return true when
	reload_in_progress is true.  Remove INT_5_BITS check.
	(floating_point_store_memory_operand): Don't return true when
	reload_in_progress is true.  Use INT14_OK_STRICT to check
	whether long displacements are always okay.

diff --git a/gcc/config/pa/pa.cc b/gcc/config/pa/pa.cc
index 218c48b4ae0..565c948a9e6 100644
--- a/gcc/config/pa/pa.cc
+++ b/gcc/config/pa/pa.cc
@@ -10819,23 +10819,29 @@ pa_legitimate_address_p (machine_mode mode, rtx x, bool strict, code_helper)
 
       if (GET_CODE (index) == CONST_INT)
 	{
+	  /* Short 5-bit displacements always okay.  */
 	  if (INT_5_BITS (index))
 	    return true;
 
-	  /* When INT14_OK_STRICT is false, a secondary reload is needed
-	     to adjust the displacement of SImode and DImode floating point
-	     instructions but this may fail when the register also needs
-	     reloading.  So, we return false when STRICT is true.  We
-	     also reject long displacements for float mode addresses since
-	     the majority of accesses will use floating point instructions
-	     that don't support 14-bit offsets.  */
-	  if (!INT14_OK_STRICT
-	      && (strict || !(reload_in_progress || reload_completed))
-	      && mode != QImode
-	      && mode != HImode)
+	  if (!base14_operand (index, mode))
 	    return false;
 
-	  return base14_operand (index, mode);
+	  /* Long 14-bit displacements always okay for these cases.  */
+	  if (INT14_OK_STRICT
+	      || mode == QImode
+	      || mode == HImode)
+	    return true;
+
+	  /* A secondary reload may be needed to adjust the displacement
+	     of floating-point accesses when STRICT is nonzero.  */
+	  if (strict)
+	    return false;
+
+	  /* We get significantly better code if we allow long displacements
+	     before reload for all accesses.  Instructions must satisfy their
+	     constraints after reload, so we must have an integer access.
+	     Return true for both cases.  */
+	  return true;
 	}
 
       if (!TARGET_DISABLE_INDEXING
diff --git a/gcc/config/pa/pa.h b/gcc/config/pa/pa.h
index e65af522966..aba2cec7357 100644
--- a/gcc/config/pa/pa.h
+++ b/gcc/config/pa/pa.h
@@ -37,6 +37,11 @@ extern unsigned long total_code_bytes;
 #define TARGET_ELF32 0
 #endif
 
+/* Generate code for ELF64 ABI.  */
+#ifndef TARGET_ELF64
+#define TARGET_ELF64 0
+#endif
+
 /* Generate code for SOM 32bit ABI.  */
 #ifndef TARGET_SOM
 #define TARGET_SOM 0
@@ -823,12 +828,11 @@ extern int may_call_alloca;
 
 /* Nonzero if 14-bit offsets can be used for all loads and stores.
    This is not possible when generating PA 1.x code as floating point
-   loads and stores only support 5-bit offsets.  Note that we do not
-   forbid the use of 14-bit offsets for integer modes.  Instead, we
-   use secondary reloads to fix REG+D memory addresses for integer
-   mode floating-point loads and stores.
+   accesses only support 5-bit offsets.  Note that we do not forbid
+   the use of 14-bit offsets prior to reload.  Instead, we use secondary
+   reloads to fix REG+D memory addresses for floating-point accesses.
 
-   FIXME: the ELF32 linker clobbers the LSB of the FP register number
+   FIXME: the GNU ELF linker clobbers the LSB of the FP register number
    in PA 2.0 floating-point insns with long displacements.  This is
    because R_PARISC_DPREL14WR and other relocations like it are not
    yet supported by GNU ld.  For now, we reject long displacements
@@ -836,7 +840,7 @@ extern int may_call_alloca;
 
 #define INT14_OK_STRICT \
   (TARGET_SOFT_FLOAT                                                   \
-   || (TARGET_PA_20 && !TARGET_ELF32))
+   || (TARGET_PA_20 && !TARGET_ELF32 && !TARGET_ELF64))
 
 /* The macros REG_OK_FOR..._P assume that the arg is a REG rtx
    and check its validity for a certain class.
diff --git a/gcc/config/pa/pa64-linux.h b/gcc/config/pa/pa64-linux.h
index f61c9e16f44..33fed023643 100644
--- a/gcc/config/pa/pa64-linux.h
+++ b/gcc/config/pa/pa64-linux.h
@@ -17,6 +17,10 @@ You should have received a copy of the GNU General Public License
 along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
+/* 64-bit ELF target.  */
+#undef TARGET_ELF64
+#define TARGET_ELF64 1
+
 #if 0 /* needs some work :-( */
 /* If defined, this macro specifies a table of register pairs used to
    eliminate unneeded registers that point into the stack frame.  */
diff --git a/gcc/config/pa/predicates.md b/gcc/config/pa/predicates.md
index 08071b2d279..1b50020e1de 100644
--- a/gcc/config/pa/predicates.md
+++ b/gcc/config/pa/predicates.md
@@ -267,6 +267,10 @@
   if (!INT_14_BITS (op))
     return false;
 
+  /* Short displacement.  */
+  if (INT_5_BITS (op))
+    return true;
+
   /* Although this may not be necessary, we require that the
      base value is correctly aligned for its mode as this is
      assumed in the instruction encoding.  */
@@ -304,15 +308,12 @@
 
   if (reg_plus_base_memory_operand (op, mode))
     {
-      if (reload_in_progress)
-	return true;
-
       /* Extract CONST_INT operand.  */
       if (GET_CODE (op) == SUBREG)
 	op = SUBREG_REG (op);
       op = XEXP (op, 0);
       op = REG_P (XEXP (op, 0)) ? XEXP (op, 1) : XEXP (op, 0);
-      return base14_operand (op, mode) || INT_5_BITS (op);
+      return base14_operand (op, mode);
     }
 
   if (!MEM_P (op))
@@ -341,17 +342,12 @@
 
   if (reg_plus_base_memory_operand (op, mode))
     {
-      if (reload_in_progress)
-	return true;
-
       /* Extract CONST_INT operand.  */
       if (GET_CODE (op) == SUBREG)
 	op = SUBREG_REG (op);
       op = XEXP (op, 0);
       op = REG_P (XEXP (op, 0)) ? XEXP (op, 1) : XEXP (op, 0);
-      return ((TARGET_PA_20
-	       && !TARGET_ELF32
-	       && base14_operand (op, mode))
+      return ((INT14_OK_STRICT && base14_operand (op, mode))
 	      || INT_5_BITS (op));
     }
 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

[Jeff Law]

On 11/16/23 10:54, John David Anglin wrote:
> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.  Committed
> to trunk.
> 
> This patch works around problem compiling python3.11 by improving
> REG+D address handling.  The change results in smaller code and
> reduced register pressure.
> 
> Dave
> ---
> 
> hppa: Revise REG+D address support to allow long displacements before reload
> 
> In analyzing PR rtl-optimization/112415, I realized that restricting
> REG+D offsets to 5-bits before reload results in very poor code and
> complexities in optimizing these instructions after reload.  The
> general problem is long displacements are not allowed for floating
> point accesses when generating PA 1.1 code.  Even with PA 2.0, there
> is a ELF linker bug that prevents using long displacements for
> floating point loads and stores.
> 
> In the past, enabling long displacements before reload caused issues
> in reload.  However, there have been fixes in the handling of reloads
> for floating-point accesses.  This change allows long displacements
> before reload and corrects a couple of issues in the constraint
> handling for integer and floating-point accesses.
> 
> 2023-11-16  John David Anglin  <danglin@gcc.gnu.org>
> 
> gcc/ChangeLog:
> 
> 	PR rtl-optimization/112415
> 	* config/pa/pa.cc (pa_legitimate_address_p): Allow 14-bit
> 	displacements before reload.  Simplify logic flow.  Revise
> 	comments.
> 	* config/pa/pa.h (TARGET_ELF64): New define.
> 	(INT14_OK_STRICT): Update define and comment.
> 	* config/pa/pa64-linux.h (TARGET_ELF64): Define.
> 	* config/pa/predicates.md (base14_operand): Don't check
> 	alignment of short displacements.
> 	(integer_store_memory_operand): Don't return true when
> 	reload_in_progress is true.  Remove INT_5_BITS check.
> 	(floating_point_store_memory_operand): Don't return true when
> 	reload_in_progress is true.  Use INT14_OK_STRICT to check
> 	whether long displacements are always okay.
I strongly suspect this is going to cause problems in the end.

I've already done what you're trying to do.  It'll likely look fine for 
an extended period of time, but it will almost certainly break one day.

Jeff

Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

[John David Anglin]

On 2023-11-16 4:52 p.m., Jeff Law wrote:
>
>
> On 11/16/23 10:54, John David Anglin wrote:
>> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.  Committed
>> to trunk.
>>
>> This patch works around problem compiling python3.11 by improving
>> REG+D address handling.  The change results in smaller code and
>> reduced register pressure.
>>
>> Dave
>> ---
>>
>> hppa: Revise REG+D address support to allow long displacements before reload
>>
>> In analyzing PR rtl-optimization/112415, I realized that restricting
>> REG+D offsets to 5-bits before reload results in very poor code and
>> complexities in optimizing these instructions after reload.  The
>> general problem is long displacements are not allowed for floating
>> point accesses when generating PA 1.1 code.  Even with PA 2.0, there
>> is a ELF linker bug that prevents using long displacements for
>> floating point loads and stores.
>>
>> In the past, enabling long displacements before reload caused issues
>> in reload.  However, there have been fixes in the handling of reloads
>> for floating-point accesses.  This change allows long displacements
>> before reload and corrects a couple of issues in the constraint
>> handling for integer and floating-point accesses.
>>
>> 2023-11-16  John David Anglin  <danglin@gcc.gnu.org>
>>
>> gcc/ChangeLog:
>>
>>     PR rtl-optimization/112415
>>     * config/pa/pa.cc (pa_legitimate_address_p): Allow 14-bit
>>     displacements before reload.  Simplify logic flow.  Revise
>>     comments.
>>     * config/pa/pa.h (TARGET_ELF64): New define.
>>     (INT14_OK_STRICT): Update define and comment.
>>     * config/pa/pa64-linux.h (TARGET_ELF64): Define.
>>     * config/pa/predicates.md (base14_operand): Don't check
>>     alignment of short displacements.
>>     (integer_store_memory_operand): Don't return true when
>>     reload_in_progress is true.  Remove INT_5_BITS check.
>>     (floating_point_store_memory_operand): Don't return true when
>>     reload_in_progress is true.  Use INT14_OK_STRICT to check
>>     whether long displacements are always okay.
> I strongly suspect this is going to cause problems in the end.
>
> I've already done what you're trying to do.  It'll likely look fine for an extended period of time, but it will almost certainly break one day.
I could happen.  If it happens and can't be fixed, it's easy enough to return false in
pa_legitimate_address_p before reload.  Maybe we could add an optimization option for this.

As it stands, the code improvement for python is significant.  I don't think f-m-o can fix things after reload.

Hopefully, Sam will test the change with various package builds on gentoo.  Debian is still on gcc-13.
I'm not seeing any obvious problems in the gcc testsuite.  It needs testing on packages that do extensive
floating point calculations.

Dave

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

Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

[Sam James]

John David Anglin <dave.anglin@bell.net> writes:

> On 2023-11-16 4:52 p.m., Jeff Law wrote:
>>
>>
>> On 11/16/23 10:54, John David Anglin wrote:
>>> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.  Committed
>>> to trunk.
>>>
>>> This patch works around problem compiling python3.11 by improving
>>> REG+D address handling.  The change results in smaller code and
>>> reduced register pressure.
>>>
>>> Dave
>>> ---
>>>
>>> hppa: Revise REG+D address support to allow long displacements before reload
>>>
>>> In analyzing PR rtl-optimization/112415, I realized that restricting
>>> REG+D offsets to 5-bits before reload results in very poor code and
>>> complexities in optimizing these instructions after reload.  The
>>> general problem is long displacements are not allowed for floating
>>> point accesses when generating PA 1.1 code.  Even with PA 2.0, there
>>> is a ELF linker bug that prevents using long displacements for
>>> floating point loads and stores.
>>>
>>> In the past, enabling long displacements before reload caused issues
>>> in reload.  However, there have been fixes in the handling of reloads
>>> for floating-point accesses.  This change allows long displacements
>>> before reload and corrects a couple of issues in the constraint
>>> handling for integer and floating-point accesses.
>>>
>>> 2023-11-16  John David Anglin  <danglin@gcc.gnu.org>
>>>
>>> gcc/ChangeLog:
>>>
>>>     PR rtl-optimization/112415
>>>     * config/pa/pa.cc (pa_legitimate_address_p): Allow 14-bit
>>>     displacements before reload.  Simplify logic flow.  Revise
>>>     comments.
>>>     * config/pa/pa.h (TARGET_ELF64): New define.
>>>     (INT14_OK_STRICT): Update define and comment.
>>>     * config/pa/pa64-linux.h (TARGET_ELF64): Define.
>>>     * config/pa/predicates.md (base14_operand): Don't check
>>>     alignment of short displacements.
>>>     (integer_store_memory_operand): Don't return true when
>>>     reload_in_progress is true.  Remove INT_5_BITS check.
>>>     (floating_point_store_memory_operand): Don't return true when
>>>     reload_in_progress is true.  Use INT14_OK_STRICT to check
>>>     whether long displacements are always okay.
>> I strongly suspect this is going to cause problems in the end.
>>
>> I've already done what you're trying to do.  It'll likely look fine
>> for an extended period of time, but it will almost certainly break
>> one day.

Jeff, I don't suppose you could dig out the old bugs/commits just out of
interest?

> I could happen.  If it happens and can't be fixed, it's easy enough to return false in
> pa_legitimate_address_p before reload.  Maybe we could add an optimization option for this.
>
> As it stands, the code improvement for python is significant.  I don't think f-m-o can fix things after reload.
>a
> Hopefully, Sam will test the change with various package builds on gentoo.  Debian is still on gcc-13.

Yeah, happy to do that. We haven't got GCC 14 deployed in the wild, but
we have it available for people who want to test and opt-in to it.

Fingers crossed it's calm. I'll let you know if it isn't ;)

> I'm not seeing any obvious problems in the gcc testsuite.  It needs testing on packages that do extensive
> floating point calculations.

OK, I'll focus on those.

>
> Dave

Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

[Sam James]

Sam James <sam@gentoo.org> writes:

> John David Anglin <dave.anglin@bell.net> writes:
>
>> On 2023-11-16 4:52 p.m., Jeff Law wrote:
>>>
>>>
>>> On 11/16/23 10:54, John David Anglin wrote:
>>>> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.  Committed
>>>> to trunk.
>>>>
>>>> This patch works around problem compiling python3.11 by improving
>>>> REG+D address handling.  The change results in smaller code and
>>>> reduced register pressure.
>>>>
>>>> Dave
>>>> ---
>>>>
>>>> hppa: Revise REG+D address support to allow long displacements before reload
>>>>
>>>> In analyzing PR rtl-optimization/112415, I realized that restricting
>>>> REG+D offsets to 5-bits before reload results in very poor code and
>>>> complexities in optimizing these instructions after reload.  The
>>>> general problem is long displacements are not allowed for floating
>>>> point accesses when generating PA 1.1 code.  Even with PA 2.0, there
>>>> is a ELF linker bug that prevents using long displacements for
>>>> floating point loads and stores.
>>>>
>>>> In the past, enabling long displacements before reload caused issues
>>>> in reload.  However, there have been fixes in the handling of reloads
>>>> for floating-point accesses.  This change allows long displacements
>>>> before reload and corrects a couple of issues in the constraint
>>>> handling for integer and floating-point accesses.
>>>>
>>>> 2023-11-16  John David Anglin  <danglin@gcc.gnu.org>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     PR rtl-optimization/112415
>>>>     * config/pa/pa.cc (pa_legitimate_address_p): Allow 14-bit
>>>>     displacements before reload.  Simplify logic flow.  Revise
>>>>     comments.
>>>>     * config/pa/pa.h (TARGET_ELF64): New define.
>>>>     (INT14_OK_STRICT): Update define and comment.
>>>>     * config/pa/pa64-linux.h (TARGET_ELF64): Define.
>>>>     * config/pa/predicates.md (base14_operand): Don't check
>>>>     alignment of short displacements.
>>>>     (integer_store_memory_operand): Don't return true when
>>>>     reload_in_progress is true.  Remove INT_5_BITS check.
>>>>     (floating_point_store_memory_operand): Don't return true when
>>>>     reload_in_progress is true.  Use INT14_OK_STRICT to check
>>>>     whether long displacements are always okay.
>>> I strongly suspect this is going to cause problems in the end.
>>>
>>> I've already done what you're trying to do.  It'll likely look fine
>>> for an extended period of time, but it will almost certainly break
>>> one day.
>
> Jeff, I don't suppose you could dig out the old bugs/commits just out of
> interest?
>
>> I could happen.  If it happens and can't be fixed, it's easy enough to return false in
>> pa_legitimate_address_p before reload.  Maybe we could add an optimization option for this.

I might hack in an option for local testing so I can quickly check
with/without...

>>
>> As it stands, the code improvement for python is significant.  I don't think f-m-o can fix things after reload.
>>a
>> Hopefully, Sam will test the change with various package builds on gentoo.  Debian is still on gcc-13.
>
> Yeah, happy to do that. We haven't got GCC 14 deployed in the wild, but
> we have it available for people who want to test and opt-in to it.
>
> Fingers crossed it's calm. I'll let you know if it isn't ;)
>
>> I'm not seeing any obvious problems in the gcc testsuite.  It needs testing on packages that do extensive
>> floating point calculations.
>
> OK, I'll focus on those.
>
>>
>> Dave

Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

[Jeff Law]

On 11/16/23 18:20, Sam James wrote:

> 
> Jeff, I don't suppose you could dig out the old bugs/commits just out of
> interest?
That work goes back to the early 90s when I was primarily responsible 
for the PA platform.  But the core issue hasn't changed in that not 
enough context is provided for reload to know how to deal with these 
problems.

So, digging out those testcases/codes would be quite difficult; at the 
time we didn't have standard procedures where tests were added to the 
testsuite for most changes or even discussed.


> 
>> I'm not seeing any obvious problems in the gcc testsuite.  It needs testing on packages that do extensive
>> floating point calculations.
> 
> OK, I'll focus on those.
THe more likely scenario is xmpy which is used for integer multiply, but 
the operands have to be moved into FP registers because the operation 
happens in the FPU.

jeff

Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

[John David Anglin]

On 2023-11-16 10:00 p.m., Jeff Law wrote:
>>> I'm not seeing any obvious problems in the gcc testsuite.  It needs testing on packages that do extensive
>>> floating point calculations.
>>
>> OK, I'll focus on those.
> THe more likely scenario is xmpy which is used for integer multiply, but the operands have to be moved into FP registers because the operation 
> happens in the FPU.
There are lots of xmpyu instructions in cc1 and cc1plus.  For example,

   9fee8c:       08 03 02 5c     copy r3,ret0
   9fee90:       37 9c 00 20     ldo 10(ret0),ret0
   9fee94:       27 80 10 17     fldw 0(ret0),fr23
   9fee98:       08 03 02 5c     copy r3,ret0
   9fee9c:       37 9c 00 28     ldo 14(ret0),ret0
   9feea0:       27 80 10 16     fldw 0(ret0),fr22
   9feea4:       3a f6 47 18     xmpyu fr23,fr22,fr24
   9feea8:       2f c1 12 18     fstd fr24,-10(sp)
   9feeac:       0f c1 10 9c     ldw -10(sp),ret0
   9feeb0:       0f c9 10 9d     ldw -c(sp),ret1

There are 2169 xmpyu instructions in cc1plus in my current gcc bootstrap on linux.

Dave

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

Re: [committed] hppa: Revise REG+D address support to allow long displacements before reload

[Jeff Law]

On 11/17/23 01:27, John David Anglin wrote:
> On 2023-11-16 10:00 p.m., Jeff Law wrote:
>>>> I'm not seeing any obvious problems in the gcc testsuite.  It needs testing on packages that do extensive
>>>> floating point calculations.
>>>
>>> OK, I'll focus on those.
>> THe more likely scenario is xmpy which is used for integer multiply, 
>> but the operands have to be moved into FP registers because the 
>> operation happens in the FPU.
> There are lots of xmpyu instructions in cc1 and cc1plus.  For example,
> 
>    9fee8c:       08 03 02 5c     copy r3,ret0
>    9fee90:       37 9c 00 20     ldo 10(ret0),ret0
>    9fee94:       27 80 10 17     fldw 0(ret0),fr23
>    9fee98:       08 03 02 5c     copy r3,ret0
>    9fee9c:       37 9c 00 28     ldo 14(ret0),ret0
>    9feea0:       27 80 10 16     fldw 0(ret0),fr22
>    9feea4:       3a f6 47 18     xmpyu fr23,fr22,fr24
>    9feea8:       2f c1 12 18     fstd fr24,-10(sp)
>    9feeac:       0f c1 10 9c     ldw -10(sp),ret0
>    9feeb0:       0f c9 10 9d     ldw -c(sp),ret1
> 
> There are 2169 xmpyu instructions in cc1plus in my current gcc bootstrap 
> on linux.
> 

Yea, and as I said, it'll likely work for quite a while until you get 
spills in just the right scenario.  We ran bootstraps, OS builds (our 
BSD and Mach systems), spec89, spec92, etc.  Eventually it breaks :(


jeff