Re: Please accept this commit for the trunk
Hi Mike, On Thu, Feb 08, 2018 at 06:06:05PM -0800, Mike Stump wrote: > On Feb 8, 2018, at 12:36 PM, Segher Boessenkool> wrote: > > > > On Wed, Feb 07, 2018 at 03:52:27PM -0800, Mike Stump wrote: > >> I dusted the pointed to patch off and check it in. Let us know how it > >> goes. > > > > I wanted to test this on the primary and secondary powerpc targets as > > well, but okay. > > I reviewed it, and it seemed to only trigger for darwin. Certainly doesn't > hurt to run a regression run and ensure that is the case. We'll find out if it regresses :-) > >> Does this resolve all of PR84113? If so, I can push the bug along. > > > > It makes bootstrap work. We don't know if it is correct otherwise. > > So, would be nice if someone could run a regression test. I'd do it by the > version just before the breakage, and then drop in the patch, and test again. > This minimizes all the other changes. Douglas has done a test (C family languages only it seems) at https://gcc.gnu.org/ml/gcc-testresults/2018-02/msg00374.html but it is hard to compare to previous results (not the same config, long ago, etc.) > >> What PR was the attachment url from? > > > > It is not from a PR, and it has never been sent to gcc-patches; it is > > from https://gcc.gnu.org/ml/gcc-testresults/2017-01/msg02971.html > > (attachment #2). > > Ah, that explains it. > > Sounds like 1, 3 and 4 also likely need to go it to make things nice. If > someone could regression test and let us know, that's likely the gating > factor. Yeah, it's not easy to accept those patches without proper regression testing, it's stage 4... But the patches probably are good. Segher
Re: Please accept this commit for the trunk
On Feb 8, 2018, at 12:36 PM, Segher Boessenkoolwrote: > > On Wed, Feb 07, 2018 at 03:52:27PM -0800, Mike Stump wrote: >> I dusted the pointed to patch off and check it in. Let us know how it goes. > > I wanted to test this on the primary and secondary powerpc targets as > well, but okay. I reviewed it, and it seemed to only trigger for darwin. Certainly doesn't hurt to run a regression run and ensure that is the case. >> Does this resolve all of PR84113? If so, I can push the bug along. > > It makes bootstrap work. We don't know if it is correct otherwise. So, would be nice if someone could run a regression test. I'd do it by the version just before the breakage, and then drop in the patch, and test again. This minimizes all the other changes. >> What PR was the attachment url from? > > It is not from a PR, and it has never been sent to gcc-patches; it is > from https://gcc.gnu.org/ml/gcc-testresults/2017-01/msg02971.html > (attachment #2). Ah, that explains it. Sounds like 1, 3 and 4 also likely need to go it to make things nice. If someone could regression test and let us know, that's likely the gating factor.
Re: Please accept this commit for the trunk
On Wed, Feb 07, 2018 at 03:52:27PM -0800, Mike Stump wrote: > I dusted the pointed to patch off and check it in. Let us know how it goes. I wanted to test this on the primary and secondary powerpc targets as well, but okay. > Does this resolve all of PR84113? If so, I can push the bug along. It makes bootstrap work. We don't know if it is correct otherwise. > What PR was the attachment url from? It is not from a PR, and it has never been sent to gcc-patches; it is from https://gcc.gnu.org/ml/gcc-testresults/2017-01/msg02971.html (attachment #2). It is also PR80865 btw (I'll take care of it). Thanks, Segher > 2018-02-07 Iain Sandoe> > * config/rs6000/altivec.md (*restore_world): Remove LR use. > * config/rs6000/predicates.md (restore_world_operation): Adjust op > count, remove one USE. > > Index: gcc/config/rs6000/altivec.md > === > --- gcc/config/rs6000/altivec.md (revision 257471) > +++ gcc/config/rs6000/altivec.md (working copy) > @@ -419,7 +419,6 @@ > (define_insn "*restore_world" > [(match_parallel 0 "restore_world_operation" >[(return) > -(use (reg:SI LR_REGNO)) > (use (match_operand:SI 1 "call_operand" "s")) > (clobber (match_operand:SI 2 "gpc_reg_operand" "=r"))])] > "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && TARGET_32BIT" > Index: gcc/config/rs6000/predicates.md > === > --- gcc/config/rs6000/predicates.md (revision 257471) > +++ gcc/config/rs6000/predicates.md (working copy) > @@ -1295,13 +1295,12 @@ >rtx elt; >int count = XVECLEN (op, 0); > > - if (count != 59) > + if (count != 58) > return 0; > >index = 0; >if (GET_CODE (XVECEXP (op, 0, index++)) != RETURN >|| GET_CODE (XVECEXP (op, 0, index++)) != USE > - || GET_CODE (XVECEXP (op, 0, index++)) != USE >|| GET_CODE (XVECEXP (op, 0, index++)) != CLOBBER) > return 0; >
Re: Please accept this commit for the trunk
On Feb 5, 2018, at 8:42 AM, Douglas Menckenwrote: > > I’m about > > “ [PATCH 2/4] [Darwin,PPC] Remove uses of LR in > restore_world ” https://gcc.gnu.org/bugzilla/attachment.cgi?id=42304 > > look at bug #84113 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84113 for > more info > > “ One important question ’s yet: Why this patch has been ignored despite > it’s been made just in time? ” I dusted the pointed to patch off and check it in. Let us know how it goes. Does this resolve all of PR84113? If so, I can push the bug along. What PR was the attachment url from? Thanks for your help. 2018-02-07 Iain Sandoe * config/rs6000/altivec.md (*restore_world): Remove LR use. * config/rs6000/predicates.md (restore_world_operation): Adjust op count, remove one USE. Index: gcc/config/rs6000/altivec.md === --- gcc/config/rs6000/altivec.md(revision 257471) +++ gcc/config/rs6000/altivec.md(working copy) @@ -419,7 +419,6 @@ (define_insn "*restore_world" [(match_parallel 0 "restore_world_operation" [(return) - (use (reg:SI LR_REGNO)) (use (match_operand:SI 1 "call_operand" "s")) (clobber (match_operand:SI 2 "gpc_reg_operand" "=r"))])] "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && TARGET_32BIT" Index: gcc/config/rs6000/predicates.md === --- gcc/config/rs6000/predicates.md (revision 257471) +++ gcc/config/rs6000/predicates.md (working copy) @@ -1295,13 +1295,12 @@ rtx elt; int count = XVECLEN (op, 0); - if (count != 59) + if (count != 58) return 0; index = 0; if (GET_CODE (XVECEXP (op, 0, index++)) != RETURN || GET_CODE (XVECEXP (op, 0, index++)) != USE - || GET_CODE (XVECEXP (op, 0, index++)) != USE || GET_CODE (XVECEXP (op, 0, index++)) != CLOBBER) return 0;