| Message ID | ead7afe8d9416a281c71f63638c8c359cb0c014a.1509441791.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Fix crash with big stack clash interval (PR82674) | expand |
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
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
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
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); }
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(-)