PR84300, ICE in dwarf2cfi on ppc64le

Message ID 20180209054247.GG3846@bubble.grove.modra.org
State New
Headers show
Series
  • PR84300, ICE in dwarf2cfi on ppc64le
Related show

Commit Message

Alan Modra Feb. 9, 2018, 5:42 a.m.
This PR is one of those with a really obvious cause, and fix.  There's
nothing in the unspec rtl to say the insn needs lr!
Bootstrapped and regression tested powerpc64le-linux.  OK?

	PR target/84300
gcc/
	* config/rs6000/rs6000.md (split_stack_return): Use LR.
gcc/testsuite/
	* gcc.dg/pr84300.c: New.

Comments

Segher Boessenkool Feb. 9, 2018, 2:11 p.m. | #1
On Fri, Feb 09, 2018 at 04:12:47PM +1030, Alan Modra wrote:
> This PR is one of those with a really obvious cause, and fix.  There's
> nothing in the unspec rtl to say the insn needs lr!
> Bootstrapped and regression tested powerpc64le-linux.  OK?
> 
> 	PR target/84300
> gcc/
> 	* config/rs6000/rs6000.md (split_stack_return): Use LR.
> gcc/testsuite/
> 	* gcc.dg/pr84300.c: New.
> 
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 33f0d95..287461f 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -13359,7 +13359,8 @@ (define_insn "load_split_stack_limit_si"
>  ;; Use r0 to stop regrename twiddling with lr restore insns emitted
>  ;; after the call to __morestack.
>  (define_insn "split_stack_return"
> -  [(unspec_volatile [(use (reg:SI 0))] UNSPECV_SPLIT_STACK_RETURN)]
> +  [(unspec_volatile [(use (reg:SI 0)) (use (reg:SI LR_REGNO))]
> +		    UNSPECV_SPLIT_STACK_RETURN)]

I'm not sure what a USE as input of an UNSPEC means -- it should work
without the USEs?

So please try without; otherwise okay of course.  Thanks!


Segher
Alan Modra Feb. 10, 2018, 3:39 a.m. | #2
On Fri, Feb 09, 2018 at 08:11:44AM -0600, Segher Boessenkool wrote:
> On Fri, Feb 09, 2018 at 04:12:47PM +1030, Alan Modra wrote:
> >  ;; Use r0 to stop regrename twiddling with lr restore insns emitted
> >  ;; after the call to __morestack.
> >  (define_insn "split_stack_return"
> > -  [(unspec_volatile [(use (reg:SI 0))] UNSPECV_SPLIT_STACK_RETURN)]
> > +  [(unspec_volatile [(use (reg:SI 0)) (use (reg:SI LR_REGNO))]
> > +		    UNSPECV_SPLIT_STACK_RETURN)]
> 
> I'm not sure what a USE as input of an UNSPEC means -- it should work
> without the USEs?

Hmm, yes, plain [(reg:SI 0) (reg:SI LR_REGNO)] ought to work.  A sniff
test says it's OK but I'll do the whole bootstrap/regtest cycle before
committing.  I'm not sure why I put the USE there for r0, probably
because I had the r0 dependency outside the unspec initially then
decided it could replace (const_int 0) inside the unspec vector to
save on useless RTL.  I didn't go far enough in trimming the RTL..
Segher Boessenkool Feb. 12, 2018, 3:22 p.m. | #3
On Sat, Feb 10, 2018 at 02:09:57PM +1030, Alan Modra wrote:
> On Fri, Feb 09, 2018 at 08:11:44AM -0600, Segher Boessenkool wrote:
> > On Fri, Feb 09, 2018 at 04:12:47PM +1030, Alan Modra wrote:
> > >  ;; Use r0 to stop regrename twiddling with lr restore insns emitted
> > >  ;; after the call to __morestack.
> > >  (define_insn "split_stack_return"
> > > -  [(unspec_volatile [(use (reg:SI 0))] UNSPECV_SPLIT_STACK_RETURN)]
> > > +  [(unspec_volatile [(use (reg:SI 0)) (use (reg:SI LR_REGNO))]
> > > +		    UNSPECV_SPLIT_STACK_RETURN)]
> > 
> > I'm not sure what a USE as input of an UNSPEC means -- it should work
> > without the USEs?
> 
> Hmm, yes, plain [(reg:SI 0) (reg:SI LR_REGNO)] ought to work.  A sniff
> test says it's OK but I'll do the whole bootstrap/regtest cycle before
> committing.  I'm not sure why I put the USE there for r0, probably
> because I had the r0 dependency outside the unspec initially then
> decided it could replace (const_int 0) inside the unspec vector to
> save on useless RTL.  I didn't go far enough in trimming the RTL..

For R0 you can use a USE I think, but it should be in a parallel with
the unspec then.  But LR more appropriately is an input to the unspec,
it's not just there to keep the register alive for <some reason>.

Having both as inputs works of course.


Segher

Patch

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 33f0d95..287461f 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -13359,7 +13359,8 @@  (define_insn "load_split_stack_limit_si"
 ;; Use r0 to stop regrename twiddling with lr restore insns emitted
 ;; after the call to __morestack.
 (define_insn "split_stack_return"
-  [(unspec_volatile [(use (reg:SI 0))] UNSPECV_SPLIT_STACK_RETURN)]
+  [(unspec_volatile [(use (reg:SI 0)) (use (reg:SI LR_REGNO))]
+		    UNSPECV_SPLIT_STACK_RETURN)]
   ""
   "blr"
   [(set_attr "type" "jmpreg")])
diff --git a/gcc/testsuite/gcc.dg/pr84300.c b/gcc/testsuite/gcc.dg/pr84300.c
new file mode 100644
index 0000000..6016799
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr84300.c
@@ -0,0 +1,5 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target split_stack } */
+/* { dg-options "-g -O2 -fsplit-stack -fno-omit-frame-pointer" } */
+
+void trap () { __builtin_trap (); }