Re: [PINGv3][PATCH] Fix for PR 61561
Sendinggcc/ChangeLog Sendinggcc/config/arm/thumb1.md Transmitting file data .. Committed revision 213710. P.S. Sorry for inconvenience. On 08/07/2014 01:02 PM, Marat Zakirov wrote: Sorry ;( Will test&fix it. On 08/07/2014 12:50 PM, Richard Earnshaw wrote: On 08/06/2014 06:44 PM, Richard Earnshaw wrote: Similarly for the movqi pattern. You haven't updated the thumb1 QImode pattern in the same way. R. On 07/08/14 09:10, Marat Zakirov wrote: --Marat On 08/07/2014 12:00 PM, Ramana Radhakrishnan wrote: On Thu, Aug 7, 2014 at 8:36 AM, Marat Zakirov wrote: Thank you. $ svn commit Sendinggcc/ChangeLog Sendinggcc/config/arm/thumb1.md Sending gcc/config/arm/thumb2.md./gcc/config/arm/thumb1.md Transmitting file data ... Committed revision 213695. P.S. Minor nit was reg. tested. Another minor nit - please send the patch you committed to be archived on the mailing list. regards Ramana On 08/06/2014 06:44 PM, Richard Earnshaw wrote: On 06/08/14 15:14, Ramana Radhakrishnan wrote: This is OK thanks. Ramana Hmm, minor nit. (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] This would be better expressed as: [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] that is, to use the 4th alternative. That's because the use of SP in these operations does not clobber the flags. Similarly for the movqi pattern. R. thumb2.diff gcc/ChangeLog: 2014-08-07 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index cd1adf4..fed741e 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -707,8 +707,8 @@ ) (define_insn "*thumb1_movhi_insn" - [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") -(match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") +(match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") -(match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] +(match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 029a679..983b59d 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") -(match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] +(match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" gcc/ChangeLog: 2014-08-07 Marat Zakirov * config/arm/thumb1.md (*thumb1_movqi_insn): Copy of thumb1_movhi_insn. --- gcc/config/arm/thumb1.md (revision 213695) +++ gcc/config/arm/thumb1.md (working copy) @@ -761,8 +761,8 @@ ) (define_insn "*thumb1_movqi_insn" - [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] + [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") + (match_operand:QI 1 "general_operand" "l,m,l,k*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))"
Re: [PINGv3][PATCH] Fix for PR 61561
Sorry ;( Will test&fix it. On 08/07/2014 12:50 PM, Richard Earnshaw wrote: On 08/06/2014 06:44 PM, Richard Earnshaw wrote: Similarly for the movqi pattern. You haven't updated the thumb1 QImode pattern in the same way. R. On 07/08/14 09:10, Marat Zakirov wrote: --Marat On 08/07/2014 12:00 PM, Ramana Radhakrishnan wrote: On Thu, Aug 7, 2014 at 8:36 AM, Marat Zakirov wrote: Thank you. $ svn commit Sendinggcc/ChangeLog Sendinggcc/config/arm/thumb1.md Sendinggcc/config/arm/thumb2.md./gcc/config/arm/thumb1.md Transmitting file data ... Committed revision 213695. P.S. Minor nit was reg. tested. Another minor nit - please send the patch you committed to be archived on the mailing list. regards Ramana On 08/06/2014 06:44 PM, Richard Earnshaw wrote: On 06/08/14 15:14, Ramana Radhakrishnan wrote: This is OK thanks. Ramana Hmm, minor nit. (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] This would be better expressed as: [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] that is, to use the 4th alternative. That's because the use of SP in these operations does not clobber the flags. Similarly for the movqi pattern. R. thumb2.diff gcc/ChangeLog: 2014-08-07 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index cd1adf4..fed741e 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -707,8 +707,8 @@ ) (define_insn "*thumb1_movhi_insn" - [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") + (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 029a679..983b59d 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
Re: [PINGv3][PATCH] Fix for PR 61561
>>> On 08/06/2014 06:44 PM, Richard Earnshaw wrote: Similarly for the movqi pattern. You haven't updated the thumb1 QImode pattern in the same way. R. On 07/08/14 09:10, Marat Zakirov wrote: > --Marat > On 08/07/2014 12:00 PM, Ramana Radhakrishnan wrote: >> On Thu, Aug 7, 2014 at 8:36 AM, Marat Zakirov wrote: >>> Thank you. >>> >>> $ svn commit >>> Sendinggcc/ChangeLog >>> Sendinggcc/config/arm/thumb1.md >>> Sendinggcc/config/arm/thumb2.md >>> Transmitting file data ... >>> Committed revision 213695. >>> >>> P.S. >>> >>> Minor nit was reg. tested. >> Another minor nit - please send the patch you committed to be archived >> on the mailing list. >> >> regards >> Ramana >> >>> >>> On 08/06/2014 06:44 PM, Richard Earnshaw wrote: On 06/08/14 15:14, Ramana Radhakrishnan wrote: > > This is OK thanks. > > > Ramana > Hmm, minor nit. (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] This would be better expressed as: [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] that is, to use the 4th alternative. That's because the use of SP in these operations does not clobber the flags. Similarly for the movqi pattern. R. thumb2.diff gcc/ChangeLog: 2014-08-07 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index cd1adf4..fed741e 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -707,8 +707,8 @@ ) (define_insn "*thumb1_movhi_insn" - [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") + (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 029a679..983b59d 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
Re: [PINGv3][PATCH] Fix for PR 61561
--Marat On 08/07/2014 12:00 PM, Ramana Radhakrishnan wrote: On Thu, Aug 7, 2014 at 8:36 AM, Marat Zakirov wrote: Thank you. $ svn commit Sendinggcc/ChangeLog Sendinggcc/config/arm/thumb1.md Sendinggcc/config/arm/thumb2.md Transmitting file data ... Committed revision 213695. P.S. Minor nit was reg. tested. Another minor nit - please send the patch you committed to be archived on the mailing list. regards Ramana On 08/06/2014 06:44 PM, Richard Earnshaw wrote: On 06/08/14 15:14, Ramana Radhakrishnan wrote: This is OK thanks. Ramana Hmm, minor nit. (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] This would be better expressed as: [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] that is, to use the 4th alternative. That's because the use of SP in these operations does not clobber the flags. Similarly for the movqi pattern. R. gcc/ChangeLog: 2014-08-07 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index cd1adf4..fed741e 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -707,8 +707,8 @@ ) (define_insn "*thumb1_movhi_insn" - [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") + (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 029a679..983b59d 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
Re: [PINGv3][PATCH] Fix for PR 61561
On Thu, Aug 7, 2014 at 8:36 AM, Marat Zakirov wrote: > Thank you. > > $ svn commit > Sendinggcc/ChangeLog > Sendinggcc/config/arm/thumb1.md > Sendinggcc/config/arm/thumb2.md > Transmitting file data ... > Committed revision 213695. > > P.S. > > Minor nit was reg. tested. Another minor nit - please send the patch you committed to be archived on the mailing list. regards Ramana > > > On 08/06/2014 06:44 PM, Richard Earnshaw wrote: >> >> On 06/08/14 15:14, Ramana Radhakrishnan wrote: >>> >>> >>> This is OK thanks. >>> >>> >>> Ramana >>> >> >> Hmm, minor nit. >> >> (define_insn "*thumb1_movhi_insn" >> [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") >> - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] >> + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] >> >> This would be better expressed as: >> >> [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") >> (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] >> >> that is, to use the 4th alternative. That's because the use of SP in >> these operations does not clobber the flags. >> >> Similarly for the movqi pattern. >> >> R. >> >> >
Re: [PINGv3][PATCH] Fix for PR 61561
Thank you. $ svn commit Sendinggcc/ChangeLog Sendinggcc/config/arm/thumb1.md Sendinggcc/config/arm/thumb2.md Transmitting file data ... Committed revision 213695. P.S. Minor nit was reg. tested. On 08/06/2014 06:44 PM, Richard Earnshaw wrote: On 06/08/14 15:14, Ramana Radhakrishnan wrote: This is OK thanks. Ramana Hmm, minor nit. (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] This would be better expressed as: [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] that is, to use the 4th alternative. That's because the use of SP in these operations does not clobber the flags. Similarly for the movqi pattern. R.
Re: [PINGv3][PATCH] Fix for PR 61561
On 06/08/14 15:14, Ramana Radhakrishnan wrote: > > > This is OK thanks. > > > Ramana > Hmm, minor nit. (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] This would be better expressed as: [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,l*r,*h,l") (match_operand:HI 1 "general_operand" "l,m,l,k*h,*r,I"))] that is, to use the 4th alternative. That's because the use of SP in these operations does not clobber the flags. Similarly for the movqi pattern. R.
Re: [PINGv3][PATCH] Fix for PR 61561
This is OK thanks. Ramana
[PINGv3][PATCH] Fix for PR 61561
On 07/30/2014 04:56 PM, Marat Zakirov wrote: On 07/23/2014 05:33 PM, Marat Zakirov wrote: Hi all! This is a friendly reminder message. On 07/17/2014 03:22 PM, Marat Zakirov wrote: On 07/16/2014 01:32 PM, Kyrill Tkachov wrote: On 16/07/14 10:22, Marat Zakirov wrote: Christophe, Please look at a new patch. Draft tests are OK. I'll ask your commit approval when full regression (ARM/thumb1/thumb2) tests are done. Hi Marat, I was about to propose the thumb2.md hunk myself, but I'll defer to the arm maintainers to comment on the other parts. Also, in the ChangeLog it is helpful to specify which patterns are being affected, so in your case it would be something like: * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Ditto. Kyrill Christophe, Kirill, finally I've finished regression testing. Please check if my patch is OK for trunk. The following configures were used: configure --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=arm-linux-gnueabi --with-interwork --enable-long-long --enable-languages=c,c++,fortran --enable-shared --with-gnu-as --with-gnu-ld --with-arch=$ARCH --with-mode=$MODE Thumb-1 $ARCH=armv4t $MODE=thumb Thumb-2 $ARCH=armv7 $MODE=thumb ARM $ARCH=armv7-a $MODE=arm No regressions detected, test pr61561.c passed in all cases. Thank you all. --Marat gcc/ChangeLog: 2014-07-16 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index c044fd5..47b5cbd 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -708,7 +708,7 @@ (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 6ea0810..7228069 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
[PINGv2][PATCH] Fix for PR 61561
On 07/23/2014 05:33 PM, Marat Zakirov wrote: Hi all! This is a friendly reminder message. On 07/17/2014 03:22 PM, Marat Zakirov wrote: On 07/16/2014 01:32 PM, Kyrill Tkachov wrote: On 16/07/14 10:22, Marat Zakirov wrote: Christophe, Please look at a new patch. Draft tests are OK. I'll ask your commit approval when full regression (ARM/thumb1/thumb2) tests are done. Hi Marat, I was about to propose the thumb2.md hunk myself, but I'll defer to the arm maintainers to comment on the other parts. Also, in the ChangeLog it is helpful to specify which patterns are being affected, so in your case it would be something like: * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Ditto. Kyrill Christophe, Kirill, finally I've finished regression testing. Please check if my patch is OK for trunk. The following configures were used: configure --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=arm-linux-gnueabi --with-interwork --enable-long-long --enable-languages=c,c++,fortran --enable-shared --with-gnu-as --with-gnu-ld --with-arch=$ARCH --with-mode=$MODE Thumb-1 $ARCH=armv4t $MODE=thumb Thumb-2 $ARCH=armv7 $MODE=thumb ARM $ARCH=armv7-a $MODE=arm No regressions detected, test pr61561.c passed in all cases. Thank you all. --Marat gcc/ChangeLog: 2014-07-16 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index c044fd5..47b5cbd 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -708,7 +708,7 @@ (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 6ea0810..7228069 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
[PING][PATCH] Fix for PR 61561
Hi all! This is a friendly reminder message. On 07/17/2014 03:22 PM, Marat Zakirov wrote: On 07/16/2014 01:32 PM, Kyrill Tkachov wrote: On 16/07/14 10:22, Marat Zakirov wrote: Christophe, Please look at a new patch. Draft tests are OK. I'll ask your commit approval when full regression (ARM/thumb1/thumb2) tests are done. Hi Marat, I was about to propose the thumb2.md hunk myself, but I'll defer to the arm maintainers to comment on the other parts. Also, in the ChangeLog it is helpful to specify which patterns are being affected, so in your case it would be something like: * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Ditto. Kyrill Christophe, Kirill, finally I've finished regression testing. Please check if my patch is OK for trunk. The following configures were used: configure --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=arm-linux-gnueabi --with-interwork --enable-long-long --enable-languages=c,c++,fortran --enable-shared --with-gnu-as --with-gnu-ld --with-arch=$ARCH --with-mode=$MODE Thumb-1 $ARCH=armv4t $MODE=thumb Thumb-2 $ARCH=armv7 $MODE=thumb ARM $ARCH=armv7-a $MODE=arm No regressions detected, test pr61561.c passed in all cases. Thank you all. --Marat gcc/ChangeLog: 2014-07-16 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index c044fd5..47b5cbd 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -708,7 +708,7 @@ (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 6ea0810..7228069 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
Re: [PING][PATCH] Fix for PR 61561
On 07/16/2014 01:32 PM, Kyrill Tkachov wrote: On 16/07/14 10:22, Marat Zakirov wrote: Christophe, Please look at a new patch. Draft tests are OK. I'll ask your commit approval when full regression (ARM/thumb1/thumb2) tests are done. Hi Marat, I was about to propose the thumb2.md hunk myself, but I'll defer to the arm maintainers to comment on the other parts. Also, in the ChangeLog it is helpful to specify which patterns are being affected, so in your case it would be something like: * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Ditto. Kyrill Christophe, Kirill, finally I've finished regression testing. Please check if my patch is OK for trunk. The following configures were used: configure --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=arm-linux-gnueabi --with-interwork --enable-long-long --enable-languages=c,c++,fortran --enable-shared --with-gnu-as --with-gnu-ld --with-arch=$ARCH --with-mode=$MODE Thumb-1 $ARCH=armv4t $MODE=thumb Thumb-2 $ARCH=armv7 $MODE=thumb ARM $ARCH=armv7-a $MODE=arm No regressions detected, test pr61561.c passed in all cases. Thank you all. --Marat gcc/ChangeLog: 2014-07-16 Marat Zakirov * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Likewise. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index c044fd5..47b5cbd 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -708,7 +708,7 @@ (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 6ea0810..7228069 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
Re: [PING][PATCH] Fix for PR 61561
On 16/07/14 10:22, Marat Zakirov wrote: Christophe, Please look at a new patch. Draft tests are OK. I'll ask your commit approval when full regression (ARM/thumb1/thumb2) tests are done. Hi Marat, I was about to propose the thumb2.md hunk myself, but I'll defer to the arm maintainers to comment on the other parts. Also, in the ChangeLog it is helpful to specify which patterns are being affected, so in your case it would be something like: * config/arm/thumb1.md (*thumb1_movhi_insn): Handle stack pointer. (*thumb1_movqi_insn): Likewise. * config/arm/thumb2.md (*thumb2_movhi_insn): Ditto. Kyrill Thank for your attention. --Marat On 07/11/2014 11:19 PM, Christophe Lyon wrote: The new testcase causes an ICE when the compile is configured --with-thumb or when forcing -mthumb. Christophe. On 11 July 2014 11:08, Marat Zakirov wrote: Thank to you all. Committed revision 212450. --Marat Original Message Subject:Re: [PING][PATCH] Fix for PR 61561 Date: Thu, 10 Jul 2014 14:01:24 +0100 From: Ramana Radhakrishnan To: Marat Zakirov , "gcc-patches@gcc.gnu.org" CC: Richard Earnshaw , Kyrylo Tkachov , Slava Garbuzov , Yuri Gribov , "mara...@gmail.com" On 30/06/14 16:21, Marat Zakirov wrote: Thank for your attention. This is OK for trunk - Sorry about the delayed response. Ramana Marat.
Re: [PING][PATCH] Fix for PR 61561
Christophe, Please look at a new patch. Draft tests are OK. I'll ask your commit approval when full regression (ARM/thumb1/thumb2) tests are done. Thank for your attention. --Marat On 07/11/2014 11:19 PM, Christophe Lyon wrote: The new testcase causes an ICE when the compile is configured --with-thumb or when forcing -mthumb. Christophe. On 11 July 2014 11:08, Marat Zakirov wrote: Thank to you all. Committed revision 212450. --Marat Original Message Subject:Re: [PING][PATCH] Fix for PR 61561 Date: Thu, 10 Jul 2014 14:01:24 +0100 From: Ramana Radhakrishnan To: Marat Zakirov , "gcc-patches@gcc.gnu.org" CC: Richard Earnshaw , Kyrylo Tkachov , Slava Garbuzov , Yuri Gribov , "mara...@gmail.com" On 30/06/14 16:21, Marat Zakirov wrote: Thank for your attention. This is OK for trunk - Sorry about the delayed response. Ramana Marat. gcc/ChangeLog: 2014-07-16 Marat Zakirov * config/arm/thumb1.md: Handle stack pointer. * config/arm/thumb2.md: Ditto. diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index c044fd5..47b5cbd 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -708,7 +708,7 @@ (define_insn "*thumb1_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:HI 1 "general_operand" "l,m,l,*h,*r,I"))] + (match_operand:HI 1 "general_operand" "lk,m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" @@ -762,7 +762,7 @@ (define_insn "*thumb1_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l") - (match_operand:QI 1 "general_operand" "l, m,l,*h,*r,I"))] + (match_operand:QI 1 "general_operand" "lk, m,l,*h,*r,I"))] "TARGET_THUMB1 && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 6ea0810..7228069 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -318,7 +318,7 @@ ;; of the messiness associated with the ARM patterns. (define_insn "*thumb2_movhi_insn" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,l,r,m,r") - (match_operand:HI 1 "general_operand" "r,I,Py,n,r,m"))] + (match_operand:HI 1 "general_operand" "rk,I,Py,n,r,m"))] "TARGET_THUMB2 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))"
Re: [PING][PATCH] Fix for PR 61561
On 14 July 2014 09:45, Marat Zakirov wrote: > Unfortunately, I didn't reproduce the issue... > > Could you please provide full info? Including gcc trunk version, configure > line, failed test command line. > > Thank you. > I'm not at office, so I am not going to be able to provide full details. However, the trunk version is the one where you committed your patch. configure: --target arm-none-linux-gnueabihf --with-thumb should be sufficient, then just make check and look at the results of the test you introduced. Christophe. > --Marat > > > On 07/11/2014 11:19 PM, Christophe Lyon wrote: >> >> The new testcase causes an ICE when the compile is configured >> --with-thumb or when forcing -mthumb. >> >> Christophe. >> >> >> On 11 July 2014 11:08, Marat Zakirov wrote: >>> >>> Thank to you all. >>> >>> Committed revision 212450. >>> >>> --Marat >>> >>> >>> Original Message >>> Subject:Re: [PING][PATCH] Fix for PR 61561 >>> Date: Thu, 10 Jul 2014 14:01:24 +0100 >>> From: Ramana Radhakrishnan >>> To: Marat Zakirov , "gcc-patches@gcc.gnu.org" >>> >>> CC: Richard Earnshaw , Kyrylo Tkachov >>> , Slava Garbuzov , Yuri >>> Gribov , "mara...@gmail.com" >>> >>> >>> >>> >>> >>> On 30/06/14 16:21, Marat Zakirov wrote: >>>> >>>> >>>> Thank for your attention. >>> >>> >>> This is OK for trunk - Sorry about the delayed response. >>> >>> Ramana >>> >>>> Marat. >>>> >>> >>> >
Re: [PING][PATCH] Fix for PR 61561
Unfortunately, I didn't reproduce the issue... Could you please provide full info? Including gcc trunk version, configure line, failed test command line. Thank you. --Marat On 07/11/2014 11:19 PM, Christophe Lyon wrote: The new testcase causes an ICE when the compile is configured --with-thumb or when forcing -mthumb. Christophe. On 11 July 2014 11:08, Marat Zakirov wrote: Thank to you all. Committed revision 212450. --Marat Original Message Subject:Re: [PING][PATCH] Fix for PR 61561 Date: Thu, 10 Jul 2014 14:01:24 +0100 From: Ramana Radhakrishnan To: Marat Zakirov , "gcc-patches@gcc.gnu.org" CC: Richard Earnshaw , Kyrylo Tkachov , Slava Garbuzov , Yuri Gribov , "mara...@gmail.com" On 30/06/14 16:21, Marat Zakirov wrote: Thank for your attention. This is OK for trunk - Sorry about the delayed response. Ramana Marat.
Re: Re: [PING][PATCH] Fix for PR 61561
The new testcase causes an ICE when the compile is configured --with-thumb or when forcing -mthumb. Christophe. On 11 July 2014 11:08, Marat Zakirov wrote: > Thank to you all. > > Committed revision 212450. > > --Marat > > > Original Message > Subject: Re: [PING][PATCH] Fix for PR 61561 > Date: Thu, 10 Jul 2014 14:01:24 +0100 > From: Ramana Radhakrishnan > To: Marat Zakirov , "gcc-patches@gcc.gnu.org" > > CC: Richard Earnshaw , Kyrylo Tkachov > , Slava Garbuzov , Yuri > Gribov , "mara...@gmail.com" > > > > > > On 30/06/14 16:21, Marat Zakirov wrote: >> >> >> Thank for your attention. > > > This is OK for trunk - Sorry about the delayed response. > > Ramana > >> Marat. >> > > >
Re: [PATCH] Fix for PR 61561
On 19/06/14 21:19, Yuri Gribov wrote: >> Thirdly, we also need to fix movhi_bytes (for pre-v4) thumb2_movhi_insn >> (for thumb2) and, quite possibly, thumb1_movhi_insn (for thumb1). There >> may well be additional changes for movqi variants as well. > > A general question: how should one test ARM backend patches? Is it > enough to regtest ARM and Thumb2 on some modern Cortex? If not - what > other variants should be tested? > > -Y > The answer to this question is that you have to be realistic. It's clearly impossible to test every permutation, but you should, ideally, test a reasonable selection of permutations that might be affected by a change. Ie, if you make a change that is likely to affect thumb1 code generation, it doesn't make sense to only test the arm or thumb2 code generation (and vice versa). The tricky thing here is that although thumb1 and thumb2 sound quite similar in name, in practice thumb2 code generation is closer to ARM than thumb1. R.
Fwd: Re: [PING][PATCH] Fix for PR 61561
Thank to you all. Committed revision 212450. --Marat Original Message Subject:Re: [PING][PATCH] Fix for PR 61561 Date: Thu, 10 Jul 2014 14:01:24 +0100 From: Ramana Radhakrishnan To: Marat Zakirov , "gcc-patches@gcc.gnu.org" CC: Richard Earnshaw , Kyrylo Tkachov , Slava Garbuzov , Yuri Gribov , "mara...@gmail.com" On 30/06/14 16:21, Marat Zakirov wrote: Thank for your attention. This is OK for trunk - Sorry about the delayed response. Ramana Marat.
Re: [PATCH] Fix for PR 61561
On Thu, Jun 19, 2014 at 9:19 PM, Yuri Gribov wrote: >> Thirdly, we also need to fix movhi_bytes (for pre-v4) thumb2_movhi_insn >> (for thumb2) and, quite possibly, thumb1_movhi_insn (for thumb1). There >> may well be additional changes for movqi variants as well. > > A general question: how should one test ARM backend patches? Is it > enough to regtest ARM and Thumb2 on some modern Cortex? If not - what > other variants should be tested? We don't expect everyone to test every single variant as one wouldn't be able to make forward progress. 1. Cross testing armv7-a / Thumb2 on qemu is acceptable. 2. If you have the resources (now with A15's) bootstrapping and regression testing on a modern machine would be so much better. 3. Cross test on qemu for older variants that may fail- you could consider cross testing on just the older variants. Testing on qemu comes with it's pitfalls, the threaded tests are a joke / the asan tests continue to randomly fail but in general it gives you a good enough indication about how badly things maybe broken. 4. Watch out for any regressions / bug reports from folks on bugzilla / the mailing lists and deal with any fallout that comes promptly. Thanks, Ramana > > -Y
Re: [PING][PATCH] Fix for PR 61561
On 30/06/14 16:21, Marat Zakirov wrote: Thank for your attention. This is OK for trunk - Sorry about the delayed response. Ramana Marat.
[PING v2][PATCH] Fix for PR 61561
Original Message Subject:[PING][PATCH] Fix for PR 61561 Date: Mon, 30 Jun 2014 19:21:49 +0400 From: Marat Zakirov To: gcc-patches@gcc.gnu.org CC: Ramana Radhakrishnan , Richard Earnshaw , Kyrill Tkachov , Slava Garbuzov , Yuri Gribov , mara...@gmail.com This is a reminder message on fix for PR 61561 which is ICE while compiling something which is valid* 'C' code. (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). Ramana, Richard, I am very appreciated for your attention and error indication, your work made my patch simple and tiny. And I hope that this patch will save somebody's time. Additional info about the issue: By adding constrain 'k' we want to make gcc work properly with stack register in HI and QI mode. This is need because CSE an RTL phase in some cases propagates sp register directly to HI/QI memory set. Next reload an RTL phase trying to build HI/QI move sp into some general purpose register which is fail due to pattern absence. You may ask who needs part of sp register? Nevertheless it is legitimate operation and there is simple example of practical usage. Imagine you put some local variable 'a' into hash table which has < 256 bunches. In this case 'operator []' of some hash class may take QI from sp register which is directly pointing on 'a'. Patch was reg. tested on --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=arm-v7a15v5r2-linux-gnueabi for c,c++,fortran languages w/o bootstrap. * According to 'C' Standard [ISO/IEC 9899:2011] 6.3.2.3 "Any pointer type may be converted to an integer type. Except as previously specified, the result is implementation-defined". As we know 'char' and 'short' (also called 'short int') are both integers. Thank for your attention. Marat. gcc/ChangeLog: 2014-06-30 Marat Zakirov PR target/61561 * config/arm/arm.md (*movhi_insn_arch4): Handle stack pointer. (*movhi_bytes): Likewise. (*arm_movqi_insn): Likewise. gcc/testsuite/ChangeLog: 2014-06-30 Marat Zakirov PR target/61561 * gcc.dg/pr61561.c: New test. diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 42c12c8..99290dc 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -6291,7 +6291,7 @@ ;; Pattern to recognize insn generated default case above (define_insn "*movhi_insn_arch4" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r") - (match_operand:HI 1 "general_operand" "rI,K,r,mi"))] + (match_operand:HI 1 "general_operand" "rIk,K,r,mi"))] "TARGET_ARM && arm_arch4 && (register_operand (operands[0], HImode) @@ -6315,7 +6315,7 @@ (define_insn "*movhi_bytes" [(set (match_operand:HI 0 "s_register_operand" "=r,r,r") - (match_operand:HI 1 "arm_rhs_operand" "I,r,K"))] + (match_operand:HI 1 "arm_rhs_operand" "I,rk,K"))] "TARGET_ARM" "@ mov%?\\t%0, %1\\t%@ movhi @@ -6430,7 +6430,7 @@ (define_insn "*arm_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m") - (match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r"))] + (match_operand:QI 1 "general_operand" "rk,rk,I,Py,K,Uu,l,m,r"))] "TARGET_32BIT && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/testsuite/gcc.dg/pr61561.c b/gcc/testsuite/gcc.dg/pr61561.c new file mode 100644 index 000..0f4b716 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr61561.c @@ -0,0 +1,15 @@ +/* PR c/61561. */ +/* { dg-do assemble } */ +/* { dg-options " -w -O2" } */ + +int dummy (int a); + +char a; +short b; + +void mmm (void) +{ + char dyn[dummy (3)]; + a = (char)&dyn[0]; + b = (short)&dyn[0]; +}
[PING][PATCH] Fix for PR 61561
This is a reminder message on fix for PR 61561 which is ICE while compiling something which is valid* 'C' code. (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). Ramana, Richard, I am very appreciated for your attention and error indication, your work made my patch simple and tiny. And I hope that this patch will save somebody's time. Additional info about the issue: By adding constrain 'k' we want to make gcc work properly with stack register in HI and QI mode. This is need because CSE an RTL phase in some cases propagates sp register directly to HI/QI memory set. Next reload an RTL phase trying to build HI/QI move sp into some general purpose register which is fail due to pattern absence. You may ask who needs part of sp register? Nevertheless it is legitimate operation and there is simple example of practical usage. Imagine you put some local variable 'a' into hash table which has < 256 bunches. In this case 'operator []' of some hash class may take QI from sp register which is directly pointing on 'a'. Patch was reg. tested on --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=arm-v7a15v5r2-linux-gnueabi for c,c++,fortran languages w/o bootstrap. * According to 'C' Standard [ISO/IEC 9899:2011] 6.3.2.3 "Any pointer type may be converted to an integer type. Except as previously specified, the result is implementation-defined". As we know 'char' and 'short' (also called 'short int') are both integers. Thank for your attention. Marat. gcc/ChangeLog: 2014-06-30 Marat Zakirov PR target/61561 * config/arm/arm.md (*movhi_insn_arch4): Handle stack pointer. (*movhi_bytes): Likewise. (*arm_movqi_insn): Likewise. gcc/testsuite/ChangeLog: 2014-06-30 Marat Zakirov PR target/61561 * gcc.dg/pr61561.c: New test. diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 42c12c8..99290dc 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -6291,7 +6291,7 @@ ;; Pattern to recognize insn generated default case above (define_insn "*movhi_insn_arch4" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r") - (match_operand:HI 1 "general_operand" "rI,K,r,mi"))] + (match_operand:HI 1 "general_operand" "rIk,K,r,mi"))] "TARGET_ARM && arm_arch4 && (register_operand (operands[0], HImode) @@ -6315,7 +6315,7 @@ (define_insn "*movhi_bytes" [(set (match_operand:HI 0 "s_register_operand" "=r,r,r") - (match_operand:HI 1 "arm_rhs_operand" "I,r,K"))] + (match_operand:HI 1 "arm_rhs_operand" "I,rk,K"))] "TARGET_ARM" "@ mov%?\\t%0, %1\\t%@ movhi @@ -6430,7 +6430,7 @@ (define_insn "*arm_movqi_insn" [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m") - (match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r"))] + (match_operand:QI 1 "general_operand" "rk,rk,I,Py,K,Uu,l,m,r"))] "TARGET_32BIT && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" diff --git a/gcc/testsuite/gcc.dg/pr61561.c b/gcc/testsuite/gcc.dg/pr61561.c new file mode 100644 index 000..0f4b716 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr61561.c @@ -0,0 +1,15 @@ +/* PR c/61561. */ +/* { dg-do assemble } */ +/* { dg-options " -w -O2" } */ + +int dummy (int a); + +char a; +short b; + +void mmm (void) +{ + char dyn[dummy (3)]; + a = (char)&dyn[0]; + b = (short)&dyn[0]; +}
[PATCH] Fix for PR 61561
Hi all, Here's my new patch for PR 61561 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). Which fixes ICE appeared due to QI/HI pattern lack in arm.md for stack pointer register. Reg. tested on arm-v7. --Marat arm.diff Description: Binary data
Re: [PATCH] Fix for PR 61561
> Thirdly, we also need to fix movhi_bytes (for pre-v4) thumb2_movhi_insn > (for thumb2) and, quite possibly, thumb1_movhi_insn (for thumb1). There > may well be additional changes for movqi variants as well. A general question: how should one test ARM backend patches? Is it enough to regtest ARM and Thumb2 on some modern Cortex? If not - what other variants should be tested? -Y
Re: [PATCH] Fix for PR 61561
On 19/06/14 16:12, Kyrill Tkachov wrote: On 19/06/14 16:05, Marat Zakirov wrote: Hi all, Here's a patch for PR 61561 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). It fixes ICE. Thanks for your contribution. However, this is *really* not the way to submit a patch and is the sort of patch that will make me avoid reviewing it instantly. But given this is your first time .. :) Can you please explain why your fix is required ? One can understand what you are attempting to achieve which is adding constraints and alternatives for moves between sp and general purpose registers for HImode and QImode operands, but how did we get here in the first place ? My next reaction was which pass is doing this and why ? Ah, then I go and read your bug report and your submission there in the description. Now I understand. Please read - https://gcc.gnu.org/contribute.html#patches Reg. tested on arm15. What's an ARM15 ? I have never seen it or even heard about it. Presumably you mean a Cortex-A15 and that is inferred from your comments in the bugzilla report. What configuration did you test, languages ? Now on to the patch itself. gcc/ChangeLog: 2014-06-19 Marat Zakirov * config/arm/arm.md: New templates see pr61561. See Changelog formats and comments about using PR numbers in Changelogs. PR target/61561 * config/arm/arm.md (*movhi_insn_arch4): Handle stack pointer (*arm_movqi_insn): Likewise. gcc/testsuite/ChangeLog: 2014-06-19 Marat Zakirov * c-c++-common/pr61561.c: New test for pr61561. PR target/61561 * : New test. diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 42c12c8..7ed8abc 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -6290,27 +6290,31 @@ ;; Pattern to recognize insn generated default case above (define_insn "*movhi_insn_arch4" - [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r") - (match_operand:HI 1 "general_operand" "rI,K,r,mi"))] + [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r,r") + (match_operand:HI 1 "general_operand" "rI,K,r,mi,k"))] "TARGET_ARM && arm_arch4 && (register_operand (operands[0], HImode) || register_operand (operands[1], HImode))" - "@ + "@ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi - ldr%(h%)\\t%0, %1\\t%@ movhi" + ldr%(h%)\\t%0, %1\\t%@ movhi + uxth%?\\t%0, %1\\t%@ movhi" Instead replace the first source constraint with rIk rather than adding another alternative. You don't need a widening operation here. There is no need to do a zero extend here. Any widening or narrowing should be dealt with where required, and the store would remain a store byte and therefore this can be a move like the other moves between registers. This pattern matches for arm_arch4 where uxth is not a valid instruction. Further more on earlier architectures this will cause an undefined instruction trap. [(set_attr "predicable" "yes") - (set_attr "pool_range" "*,*,*,256") - (set_attr "neg_pool_range" "*,*,*,244") + (set_attr "pool_range" "*,*,*,256,*") + (set_attr "neg_pool_range" "*,*,*,244,*") (set_attr_alternative "type" [(if_then_else (match_operand 1 "const_int_operand" "") (const_string "mov_imm" ) (const_string "mov_reg")) (const_string "mvn_imm") (const_string "store1") - (const_string "load1")])] + (const_string "load1") + (if_then_else (match_operand 1 "const_int_operand" "") +(const_string "mov_imm" ) +(const_string "mov_reg"))])] This should not be needed now. This is just a move_reg. ) (define_insn "*movhi_bytes" @@ -6429,8 +6433,8 @@ ) (define_insn "*arm_movqi_insn" - [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m") - (match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r"))] + [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m,r,r") + (match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r,k,k"))] Why do you need 2 alternatives which appear to do the same thing ? Instead just do a move. "TARGET_32BIT && ( register_operand (operands[0], QImode) || register_operand (operands[1], QImode))" @@ -6443,12 +6447,14 @@ ldr%(b%)\\t%0, %1 str%(b%)\\t%1, %0 ldr%(b%)\\t%0, %1 - str%(b%)\\t%1, %0" - [(set_attr "type" "mov_reg,mov_reg,mov_imm,mov_imm,mvn_imm,load1,store1,load1,store1") + str%(b%)\\t%1, %0 + uxtb%?\\t%0, %1 + uxtb%?\\t%0, %1" + [(set_attr "type" "mov_reg,mov_reg,mov_imm,mov_imm,mvn_imm,load1,store1,load1,store1,mov_reg,mov_reg") (set_attr "predicable" "yes") -
Re: [PATCH] Fix for PR 61561
On 19/06/14 16:05, Marat Zakirov wrote: > Hi all, > > Here's a patch for PR 61561 > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). > > It fixes ICE. > > Reg. tested on arm15. > > --Marat > > > arm.md.diff.diff > > > gcc/ChangeLog: > > 2014-06-19 Marat Zakirov > > * config/arm/arm.md: New templates see pr61561. > ChangeLog entries should list the names of the patterns changed. > gcc/testsuite/ChangeLog: > > 2014-06-19 Marat Zakirov > > * c-c++-common/pr61561.c: New test for pr61561. > Not OK. I don't see why you need to zero out the top bits. Firstly, it shouldn't be necessary to have a new alternative; I see no reason why these can't use the MOV instruction in alternative 0 (just change rI to rkI). Secondly, uxt[bh] are ARMv6 and later, but this pattern needs to work for armv4 and later. Thirdly, we also need to fix movhi_bytes (for pre-v4) thumb2_movhi_insn (for thumb2) and, quite possibly, thumb1_movhi_insn (for thumb1). There may well be additional changes for movqi variants as well. R. > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > index 42c12c8..7ed8abc 100644 > --- a/gcc/config/arm/arm.md > +++ b/gcc/config/arm/arm.md > @@ -6290,27 +6290,31 @@ > > ;; Pattern to recognize insn generated default case above > (define_insn "*movhi_insn_arch4" > - [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r") > - (match_operand:HI 1 "general_operand" "rI,K,r,mi"))] > + [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r,r") > + (match_operand:HI 1 "general_operand" "rI,K,r,mi,k"))] >"TARGET_ARM > && arm_arch4 > && (register_operand (operands[0], HImode) > || register_operand (operands[1], HImode))" > - "@ > + "@ > mov%?\\t%0, %1\\t%@ movhi > mvn%?\\t%0, #%B1\\t%@ movhi > str%(h%)\\t%1, %0\\t%@ movhi > - ldr%(h%)\\t%0, %1\\t%@ movhi" > + ldr%(h%)\\t%0, %1\\t%@ movhi > + uxth%?\\t%0, %1\\t%@ movhi" >[(set_attr "predicable" "yes") > - (set_attr "pool_range" "*,*,*,256") > - (set_attr "neg_pool_range" "*,*,*,244") > + (set_attr "pool_range" "*,*,*,256,*") > + (set_attr "neg_pool_range" "*,*,*,244,*") > (set_attr_alternative "type" > [(if_then_else (match_operand 1 "const_int_operand" > "") > (const_string "mov_imm" ) > (const_string "mov_reg")) >(const_string "mvn_imm") >(const_string "store1") > - (const_string "load1")])] > + (const_string "load1") > + (if_then_else (match_operand 1 "const_int_operand" > "") > +(const_string "mov_imm" ) > +(const_string "mov_reg"))])] > ) > > (define_insn "*movhi_bytes" > @@ -6429,8 +6433,8 @@ > ) > > (define_insn "*arm_movqi_insn" > - [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m") > - (match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r"))] > + [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m,r,r") > + (match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r,k,k"))] >"TARGET_32BIT > && ( register_operand (operands[0], QImode) > || register_operand (operands[1], QImode))" > @@ -6443,12 +6447,14 @@ > ldr%(b%)\\t%0, %1 > str%(b%)\\t%1, %0 > ldr%(b%)\\t%0, %1 > - str%(b%)\\t%1, %0" > - [(set_attr "type" > "mov_reg,mov_reg,mov_imm,mov_imm,mvn_imm,load1,store1,load1,store1") > + str%(b%)\\t%1, %0 > + uxtb%?\\t%0, %1 > + uxtb%?\\t%0, %1" > + [(set_attr "type" > "mov_reg,mov_reg,mov_imm,mov_imm,mvn_imm,load1,store1,load1,store1,mov_reg,mov_reg") > (set_attr "predicable" "yes") > - (set_attr "predicable_short_it" "yes,yes,yes,no,no,no,no,no,no") > - (set_attr "arch" "t2,any,any,t2,any,t2,t2,any,any") > - (set_attr "length" "2,4,4,2,4,2,2,4,4")] > + (set_attr "predicable_short_it" "yes,yes,yes,no,no,no,no,no,no,no,no") > + (set_attr "arch" "t2,any,any,t2,any,t2,t2,any,any,any,t2") > + (set_attr "length" "2,4,4,2,4,2,2,4,4,4,2")] > ) > > ;; HFmode moves > diff --git a/gcc/testsuite/c-c++-common/pr61561.c > b/gcc/testsuite/c-c++-common/pr61561.c > new file mode 100644 > index 000..0f4b716 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr61561.c > @@ -0,0 +1,15 @@ > +/* PR c/61561 */ > +/* { dg-do assemble } */ > +/* { dg-options " -w" } */ > + > +int dummy(int a); > + > +char a; > +short b; > + > +void mmm (void) > +{ > + char dyn[ dummy(3) ]; > + a = (char)&dyn[0]; > + b = (short)&dyn[0]; > +} >
Re: [PATCH] Fix for PR 61561
> + (if_then_else (match_operand 1 "const_int_operand" > "") > +(const_string "mov_imm" ) > +(const_string "mov_reg"))])] Why not just mov_reg? > * config/arm/arm.md: New templates see pr61561. I think you shouldn't reference PR in file change description, just add a general comment "PR arm/61561" to each ChangeLog (see existing ChangeLog entries). > * c-c++-common/pr61561.c: New test for pr61561. Likewise. > - "@ > + "@ Accidental change? -Y
Re: [PATCH] Fix for PR 61561
On 19/06/14 16:05, Marat Zakirov wrote: Hi all, Here's a patch for PR 61561 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). It fixes ICE. Reg. tested on arm15. CC'ing the arm maintainers... Kyrill --Marat
[PATCH] Fix for PR 61561
Hi all, Here's a patch for PR 61561 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). It fixes ICE. Reg. tested on arm15. --Marat arm.md.diff.diff Description: Binary data