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 Thu, Aug 7, 2014 at 8:36 AM, Marat Zakirov m.zaki...@samsung.com 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
--Marat On 08/07/2014 12:00 PM, Ramana Radhakrishnan wrote: On Thu, Aug 7, 2014 at 8:36 AM, Marat Zakirov m.zaki...@samsung.com 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 m.zaki...@samsung.com * 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 m.zaki...@samsung.com 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 m.zaki...@samsung.com * 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
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 testfix 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 m.zaki...@samsung.com 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 m.zaki...@samsung.com * 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 m.zaki...@samsung.com * 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))
[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 m.zaki...@samsung.com * 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: [PINGv3][PATCH] Fix for PR 61561
This is OK thanks. Ramana
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.
[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 m.zaki...@samsung.com * 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 m.zaki...@samsung.com * 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 m.zaki...@samsung.com * 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
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 m.zaki...@samsung.com 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 ramana.radhakrish...@arm.com To: Marat Zakirov m.zaki...@samsung.com, gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org CC: Richard Earnshaw richard.earns...@arm.com, Kyrylo Tkachov kyrylo.tkac...@arm.com, Slava Garbuzov v.garbu...@samsung.com, Yuri Gribov tetra2...@gmail.com, mara...@gmail.com 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 m.zaki...@samsung.com * 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 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 m.zaki...@samsung.com 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 ramana.radhakrish...@arm.com To: Marat Zakirov m.zaki...@samsung.com, gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org CC: Richard Earnshaw richard.earns...@arm.com, Kyrylo Tkachov kyrylo.tkac...@arm.com, Slava Garbuzov v.garbu...@samsung.com, Yuri Gribov tetra2...@gmail.com, mara...@gmail.com 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 m.zaki...@samsung.com 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 ramana.radhakrish...@arm.com To: Marat Zakirov m.zaki...@samsung.com, gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org CC: Richard Earnshaw richard.earns...@arm.com, Kyrylo Tkachov kyrylo.tkac...@arm.com, Slava Garbuzov v.garbu...@samsung.com, Yuri Gribov tetra2...@gmail.com, mara...@gmail.com 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
On 14 July 2014 09:45, Marat Zakirov m.zaki...@samsung.com 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 m.zaki...@samsung.com 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 ramana.radhakrish...@arm.com To: Marat Zakirov m.zaki...@samsung.com, gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org CC: Richard Earnshaw richard.earns...@arm.com, Kyrylo Tkachov kyrylo.tkac...@arm.com, Slava Garbuzov v.garbu...@samsung.com, Yuri Gribov tetra2...@gmail.com, mara...@gmail.com 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.
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 ramana.radhakrish...@arm.com To: Marat Zakirov m.zaki...@samsung.com, gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org CC: Richard Earnshaw richard.earns...@arm.com, Kyrylo Tkachov kyrylo.tkac...@arm.com, Slava Garbuzov v.garbu...@samsung.com, Yuri Gribov tetra2...@gmail.com, mara...@gmail.com 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.
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 m.zaki...@samsung.com 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 ramana.radhakrish...@arm.com To: Marat Zakirov m.zaki...@samsung.com, gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org CC: Richard Earnshaw richard.earns...@arm.com, Kyrylo Tkachov kyrylo.tkac...@arm.com, Slava Garbuzov v.garbu...@samsung.com, Yuri Gribov tetra2...@gmail.com, mara...@gmail.com 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.
[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 m.zaki...@samsung.com
To: gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org
CC: Ramana Radhakrishnan ramana.radhakrish...@arm.com, Richard
Earnshaw richard.earns...@arm.com, Kyrill Tkachov
kyrylo.tkac...@arm.com, Slava Garbuzov v.garbu...@samsung.com, Yuri
Gribov tetra2...@gmail.com, 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 m.zaki...@samsung.com
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 m.zaki...@samsung.com
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];
+}
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.
Re: [PATCH] Fix for PR 61561
On Thu, Jun 19, 2014 at 9:19 PM, Yuri Gribov tetra2...@gmail.com 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
[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 m.zaki...@samsung.com
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 m.zaki...@samsung.com
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
[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
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
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.
--Marat
arm.md.diff.diff
gcc/ChangeLog:
2014-06-19 Marat Zakirov m.zaki...@samsung.com
* 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 m.zaki...@samsung.com
* 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
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 m.zaki...@samsung.com * config/arm/arm.md: New templates see pr61561. See Changelog formats and comments about using PR numbers in Changelogs. DATE NAME email 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 m.zaki...@samsung.com * c-c++-common/pr61561.c: New test for pr61561. PR target/61561 * file_name: 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) - (set_attr predicable_short_it
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
