Re: Please accept this commit for the trunk

[Segher Boessenkool]

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

[Mike Stump]

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.

>> 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

[Segher Boessenkool]

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;
>  

Fwd: Please accept this commit for the trunk

[Douglas Mencken]

> Does this resolve all of PR84113?  If so, I can push the bug along

Yep, as I wrote in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84113#c35

> What PR was the attachment url from?

It took me about one minute to figure out and find the bug with patch
mentioned by Segher in https://gcc.gnu.org/bugzilla/
show_bug.cgi?id=84113#c25

Please test https://gcc.gnu.org/ml/gcc-testresults/2017-01/txtnZhWiDkC4z.txtg
 


On Wed, Feb 7, 2018 at 6:52 PM, Mike Stump  wrote:

> On Feb 5, 2018, at 8:42 AM, Douglas Mencken  wrote:
> >
> > 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.
>
>

Re: Please accept this commit for the trunk

[Mike Stump]

On Feb 5, 2018, at 8:42 AM, Douglas Mencken  wrote:
> 
> 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;
 

Please accept this commit for the trunk

[Douglas Mencken]

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? ”

my 0.04 gccçoins
Douglas