[PATCH] rs6000: Fix crash with big stack clash interval (PR82674)

[PATCH] rs6000: Fix crash with big stack clash interval (PR82674)

[Segher Boessenkool]

If the user asks for a stack clash probe interval of 64kB, we currently
generate a "stdu rX,-65536(r1)" instruction.  That instruction does not
exist (the offset is a 16-bit signed number).  If the offset is too big
we should force it into a register and generate a "stdux rX,rY,r1"
instruction, instead.

Bootstrapped and regression checked on powerpc64-linux {-m32,-m64};
committing to trunk.


Segher


2017-10-31  Segher Boessenkool  <segher@kernel.crsahing.org>

	PR target/82674
	* config/rs6000/rs6000.md (allocate_stack): Force update interval
	into a register if it does not fit into an immediate offset field.

---
 gcc/config/rs6000/rs6000.md | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 62bd19b..18ebe8f 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -10333,6 +10333,9 @@ (define_expand "allocate_stack"
 	{
 	  rtx loop_lab, end_loop;
 	  bool rotated = CONST_INT_P (rounded_size);
+	  rtx update = GEN_INT (-probe_interval);
+	  if (probe_interval > 32768)
+	    update = force_reg (Pmode, update);
 
 	  emit_stack_clash_protection_probe_loop_start (&loop_lab, &end_loop,
 							last_addr, rotated);
@@ -10340,13 +10343,11 @@ (define_expand "allocate_stack"
 	  if (Pmode == SImode)
 	    emit_insn (gen_movsi_update_stack (stack_pointer_rtx,
 					       stack_pointer_rtx,
-					       GEN_INT (-probe_interval),
-					       chain));
+					       update, chain));
 	  else
 	    emit_insn (gen_movdi_di_update_stack (stack_pointer_rtx,
 					          stack_pointer_rtx,
-					          GEN_INT (-probe_interval),
-					          chain));
+					          update, chain));
 	  emit_stack_clash_protection_probe_loop_end (loop_lab, end_loop,
 						      last_addr, rotated);
 	}
-- 
1.8.3.1

Re: [PATCH] rs6000: Fix crash with big stack clash interval (PR82674)

[Jeff Law]

On 10/31/2017 03:41 AM, Segher Boessenkool wrote:
> If the user asks for a stack clash probe interval of 64kB, we currently
> generate a "stdu rX,-65536(r1)" instruction.  That instruction does not
> exist (the offset is a 16-bit signed number).  If the offset is too big
> we should force it into a register and generate a "stdux rX,rY,r1"
> instruction, instead.
> 
> Bootstrapped and regression checked on powerpc64-linux {-m32,-m64};
> committing to trunk.
> 
> 
> Segher
> 
> 
> 2017-10-31  Segher Boessenkool  <segher@kernel.crsahing.org>
> 
> 	PR target/82674
> 	* config/rs6000/rs6000.md (allocate_stack): Force update interval
> 	into a register if it does not fit into an immediate offset field.
:-)  That's one I had a fix for.  But the issues with large probing
intervals are much more serious.  In particular dealing with large
probing intervals for allocations in the prologue is a significantly
tougher problem due to the lack of a free hard reg.

jeff

Re: [PATCH] rs6000: Fix crash with big stack clash interval (PR82674)

[Segher Boessenkool]

On Tue, Oct 31, 2017 at 09:12:25AM -0600, Jeff Law wrote:
> On 10/31/2017 03:41 AM, Segher Boessenkool wrote:
> > If the user asks for a stack clash probe interval of 64kB, we currently
> > generate a "stdu rX,-65536(r1)" instruction.  That instruction does not
> > exist (the offset is a 16-bit signed number).  If the offset is too big
> > we should force it into a register and generate a "stdux rX,rY,r1"
> > instruction, instead.
> > 
> > Bootstrapped and regression checked on powerpc64-linux {-m32,-m64};
> > committing to trunk.
> > 
> > 
> > Segher
> > 
> > 
> > 2017-10-31  Segher Boessenkool  <segher@kernel.crsahing.org>
> > 
> > 	PR target/82674
> > 	* config/rs6000/rs6000.md (allocate_stack): Force update interval
> > 	into a register if it does not fit into an immediate offset field.
> :-)  That's one I had a fix for.  But the issues with large probing
> intervals are much more serious.  In particular dealing with large
> probing intervals for allocations in the prologue is a significantly
> tougher problem due to the lack of a free hard reg.

Yes, but I haven't seen testcases for that?  The PR was just this.

We do have more than one register we can use here, FWIW.  It gets a
bit nasty because it depends on which ABI you're on exactly, which.


Segher

Re: [PATCH] rs6000: Fix crash with big stack clash interval (PR82674)

[Jeff Law]

On 10/31/2017 09:32 AM, Segher Boessenkool wrote:
> On Tue, Oct 31, 2017 at 09:12:25AM -0600, Jeff Law wrote:
>> On 10/31/2017 03:41 AM, Segher Boessenkool wrote:
>>> If the user asks for a stack clash probe interval of 64kB, we currently
>>> generate a "stdu rX,-65536(r1)" instruction.  That instruction does not
>>> exist (the offset is a 16-bit signed number).  If the offset is too big
>>> we should force it into a register and generate a "stdux rX,rY,r1"
>>> instruction, instead.
>>>
>>> Bootstrapped and regression checked on powerpc64-linux {-m32,-m64};
>>> committing to trunk.
>>>
>>>
>>> Segher
>>>
>>>
>>> 2017-10-31  Segher Boessenkool  <segher@kernel.crsahing.org>
>>>
>>> 	PR target/82674
>>> 	* config/rs6000/rs6000.md (allocate_stack): Force update interval
>>> 	into a register if it does not fit into an immediate offset field.
>> :-)  That's one I had a fix for.  But the issues with large probing
>> intervals are much more serious.  In particular dealing with large
>> probing intervals for allocations in the prologue is a significantly
>> tougher problem due to the lack of a free hard reg.
> 
> Yes, but I haven't seen testcases for that?  The PR was just this.
 :-)  I just turned stack clash on by default, cranked up the probing
interval and ran the testsuite and started looking at the root cause of
the failures.


> 
> We do have more than one register we can use here, FWIW.  It gets a
> bit nasty because it depends on which ABI you're on exactly, which.
BIg sigh...  There are times I wonder if I'd be happier as an app
developer or database junkie.  Instead I chose to do low level stuff :(



jeff