diff mbox series

[committed] hppa: Fix handling of unscaled index addresses on HP-UX

Message ID ZtCeO65odx6vyEf4@mx3210.local
State New
Headers show
Series [committed] hppa: Fix handling of unscaled index addresses on HP-UX | expand

Commit Message

John David Anglin Aug. 29, 2024, 4:13 p.m. UTC 
Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.
Committed to trunk.

Dave
---

hppa: Fix handling of unscaled index addresses on HP-UX

The PA-RISC architecture uses the top two bits of memory pointers
to select space registers.  The space register ID is ored with the
pointer offset to compute the global virtual address for an access.

The new late combine passes broke gcc on HP-UX.  One of these passes
runs after reload.  The existing code assumed no unscaled index
instructions would be created after reload as the REG_POINTER flag
is not reliable after reload.  The new pass sometimes interchanged
the base and index registers, causing these instructions to fault
when the wrong space register was selected.

I investigated various alternatives to try to retain generation
of unscaled index instructions on HP-UX.  It's not possible to
simply treat unscaled index addresses as not legitimate after
reload as sometimes instructions need to be rerecognized after
reload.  So, we needed to allow unscaled index addresses after
reload and to disable the late combine passes.

I had noticed that reversing the current order of base and index
register canonicalization resulted in more accesses using unscaled
index addresses.  However, this exposed issues with the REG_POINTER
flag.

The flag is not propagated when a constant is added to a pointer.
Tree opimization sometimes adds two pointers.  I found that I had
to treat the result as a pointer but the addition generally corrupts
the space register bits.  These get fixed when a negative pointer
is added.  Finally, the REG_POINTER flag isn't set when a pointer
is passed in a function call.  I couldn't get this approach to work.

Thus, I came to the conclusion that the best approach was to
disable use of unscaled index addresses on HP-UX.  I don't think
this impacts performance significantly.  Code size might get
slightly larger but we get some or more back from having the late
combine passes.

2024-08-29  John David Anglin  <danglin@gcc.gnu.org>

gcc/ChangeLog:

	* config/pa/pa.cc (load_reg): Don't generate load with
	unscaled index address when !TARGET_NO_SPACE_REGS.
	(pa_legitimate_address_p): Only allow unscaled index
	addresses when TARGET_NO_SPACE_REGS.

Comments

Jeff Law Aug. 29, 2024, 7:24 p.m. UTC | #1 
On 8/29/24 10:13 AM, John David Anglin wrote:
> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.
> Committed to trunk.
> 
> Dave
> ---
> 
> hppa: Fix handling of unscaled index addresses on HP-UX
> 
> The PA-RISC architecture uses the top two bits of memory pointers
> to select space registers.  The space register ID is ored with the
> pointer offset to compute the global virtual address for an access.
> 
> The new late combine passes broke gcc on HP-UX.  One of these passes
> runs after reload.  The existing code assumed no unscaled index
> instructions would be created after reload as the REG_POINTER flag
> is not reliable after reload.  The new pass sometimes interchanged
> the base and index registers, causing these instructions to fault
> when the wrong space register was selected.
Implicit space register selection based on the index rather than the 
effective address.   A thorn in my side for years working on the PA.

> 
> The flag is not propagated when a constant is added to a pointer.
IIRC one of the problems in this space was Ada would tend to create 
pointers outside an object's bounds, then use an offset to bring the 
effective address back into the object.  And tail merging would tend to 
do things like see a path with a + b and another with b + a as addresses 
and assume it could commonize the paths.  All kinds of dragons in here. 
I tried damn hard to support unscaled indexing, but it was always quite 
painful.


> Thus, I came to the conclusion that the best approach was to
> disable use of unscaled index addresses on HP-UX.  I don't think
> this impacts performance significantly.  Code size might get
> slightly larger but we get some or more back from having the late
> combine passes.
I can certainly live with this as I don't have to use HPUX anymore ;-) 
Presumably Linux has a flat address model with the space registers 
holding the same value.  That's what we did with our BSD and Mach ports 
to the PA and was what PRO was recommending to members back in the day...

It may be worth noting that if anyone still has mn10300 silicon it had 
similar properties.  Though it may have been limited to the first 
generation, I vaguely recall a Matsushita engineer I worked with 
indicating they considered it a silicon bug.

jeff
Richard Sandiford Aug. 30, 2024, 3:54 p.m. UTC | #2 
Jeff Law <jeffreyalaw@gmail.com> writes:
> On 8/29/24 10:13 AM, John David Anglin wrote:
>> Tested on hppa-unknown-linux-gnu and hppa64-hp-hpux11.11.
>> Committed to trunk.
>> 
>> Dave
>> ---
>> 
>> hppa: Fix handling of unscaled index addresses on HP-UX
>> 
>> The PA-RISC architecture uses the top two bits of memory pointers
>> to select space registers.  The space register ID is ored with the
>> pointer offset to compute the global virtual address for an access.
>> 
>> The new late combine passes broke gcc on HP-UX.  One of these passes
>> runs after reload.  The existing code assumed no unscaled index
>> instructions would be created after reload as the REG_POINTER flag
>> is not reliable after reload.  The new pass sometimes interchanged
>> the base and index registers, causing these instructions to fault
>> when the wrong space register was selected.
> Implicit space register selection based on the index rather than the 
> effective address.   A thorn in my side for years working on the PA.
>
>> 
>> The flag is not propagated when a constant is added to a pointer.
> IIRC one of the problems in this space was Ada would tend to create 
> pointers outside an object's bounds, then use an offset to bring the 
> effective address back into the object.  And tail merging would tend to 
> do things like see a path with a + b and another with b + a as addresses 
> and assume it could commonize the paths.  All kinds of dragons in here. 
> I tried damn hard to support unscaled indexing, but it was always quite 
> painful.
>
>
>> Thus, I came to the conclusion that the best approach was to
>> disable use of unscaled index addresses on HP-UX.  I don't think
>> this impacts performance significantly.  Code size might get
>> slightly larger but we get some or more back from having the late
>> combine passes.
> I can certainly live with this as I don't have to use HPUX anymore ;-) 
> Presumably Linux has a flat address model with the space registers 
> holding the same value.  That's what we did with our BSD and Mach ports 
> to the PA and was what PRO was recommending to members back in the day...
>
> It may be worth noting that if anyone still has mn10300 silicon it had 
> similar properties.  Though it may have been limited to the first 
> generation, I vaguely recall a Matsushita engineer I worked with 
> indicating they considered it a silicon bug.

Can't remember if I've mentioned this before, but FWIW: as part
of the Morello port, it was vital for correctness that we could
distinguish bases from indices.  We ended up adding a new mode class
for pointers (used only on targets that need them).  We also added
pointer_plus and pointer_diff rtx codes, along the lines of the
tree/gimple operations (again only used on targets that need them).
We added abstractions that were supposed to make it easy for
target-independent code to do the right thing.

For real Morello, the pointer capabilities were 128 bits, and so they
were easy to tell apart from indices.  However, for testing purposes,
we also had a "fake capability" mode that used the Morello-style
representation for normal AArch64 code, with 64-bit pointers.
It seemed to work pretty well.  Something like the fake capability
approach might be useful for targets like PA.

Morello was an experimental architecture, so there are no current
plans to submit support for trunk.  It's available in
vendors/ARM/heads/morello for anyone's who curious though.

(As always, there was quite a bit of learning by doing, so we'd need
to go back and clean the branch up if we did ever submit parts to trunk.)

Richard
diff mbox series

Patch

diff --git a/gcc/config/pa/pa.cc b/gcc/config/pa/pa.cc
index 911b7d96e9b..297ec3a6f69 100644
--- a/gcc/config/pa/pa.cc
+++ b/gcc/config/pa/pa.cc
@@ -4517,7 +4517,7 @@  load_reg (int reg, HOST_WIDE_INT disp, int base)
       rtx tmpreg = gen_rtx_REG (Pmode, 1);
 
       emit_move_insn (tmpreg, delta);
-      if (TARGET_DISABLE_INDEXING)
+      if (!TARGET_NO_SPACE_REGS || TARGET_DISABLE_INDEXING)
 	{
 	  emit_move_insn (tmpreg, gen_rtx_PLUS (Pmode, tmpreg, basereg));
 	  src = gen_rtx_MEM (word_mode, tmpreg);
@@ -11009,17 +11009,13 @@  pa_legitimate_address_p (machine_mode mode, rtx x, bool strict, code_helper)
 	}
 
       if (!TARGET_DISABLE_INDEXING
-	  /* Only accept the "canonical" INDEX+BASE operand order
-	     on targets with non-equivalent space registers.  */
-	  && (TARGET_NO_SPACE_REGS
-	      ? REG_P (index)
-	      : (base == XEXP (x, 1) && REG_P (index)
-		 && (reload_completed
-		     || (reload_in_progress && HARD_REGISTER_P (base))
-		     || REG_POINTER (base))
-		 && (reload_completed
-		     || (reload_in_progress && HARD_REGISTER_P (index))
-		     || !REG_POINTER (index))))
+	  /* Currently, the REG_POINTER flag is not set in a variety
+	     of situations (e.g., call arguments and pointer arithmetic).
+	     As a result, we can't reliably determine when unscaled
+	     addresses are legitimate on targets that need space register
+	     selection.  */
+	  && TARGET_NO_SPACE_REGS
+	  && REG_P (index)
 	  && MODE_OK_FOR_UNSCALED_INDEXING_P (mode)
 	  && (strict ? STRICT_REG_OK_FOR_INDEX_P (index)
 		     : REG_OK_FOR_INDEX_P (index))