| Message ID | 0ce9ef69-cf59-075e-d392-f5bed829c4d8@foss.arm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 09/11/16 16:19, Thomas Preudhomme wrote: > Hi, > > This patch fixes the following ICE when building when compiling an empty FIQ interrupt handler in ARM mode: > > empty_fiq_handler.c:5:1: error: insn does not satisfy its constraints: > } > ^ > > (insn/f 13 12 14 (set (reg/f:SI 13 sp) > (plus:SI (reg/f:SI 11 fp) > (const_int 4 [0x4]))) irq.c:5 4 {*arm_addsi3} > (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:SI 13 sp) > (plus:SI (reg/f:SI 11 fp) > (const_int 4 [0x4]))) > (nil))) > > The ICE was provoked by missing an alternative to reflect that ARM mode can do an add of general register into sp which is unpredictable in Thumb mode add immediate. > > ChangeLog entries are as follow: > > *** gcc/ChangeLog *** > > 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/arm.md (arm_addsi3): Add alternative for addition of > general register with general register or ARM constant into SP > register. > > > *** gcc/testsuite/ChangeLog *** > > 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * gcc.target/arm/empty_fiq_handler.c: New test. > > > Testing: bootstrapped on ARMv7-A ARM mode & testsuite shows no regression. > > Is this ok for trunk? > I see that "r" does not include the stack pointer (STACK_REG is separate from GENERAL_REGs) so we are indeed missing that constraint. Ok for trunk. Thanks, Kyrill > Best regards, > > Thomas
On 16 November 2016 at 10:39, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > > On 09/11/16 16:19, Thomas Preudhomme wrote: >> >> Hi, >> >> This patch fixes the following ICE when building when compiling an empty >> FIQ interrupt handler in ARM mode: >> >> empty_fiq_handler.c:5:1: error: insn does not satisfy its constraints: >> } >> ^ >> >> (insn/f 13 12 14 (set (reg/f:SI 13 sp) >> (plus:SI (reg/f:SI 11 fp) >> (const_int 4 [0x4]))) irq.c:5 4 {*arm_addsi3} >> (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:SI 13 sp) >> (plus:SI (reg/f:SI 11 fp) >> (const_int 4 [0x4]))) >> (nil))) >> >> The ICE was provoked by missing an alternative to reflect that ARM mode >> can do an add of general register into sp which is unpredictable in Thumb >> mode add immediate. >> >> ChangeLog entries are as follow: >> >> *** gcc/ChangeLog *** >> >> 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * config/arm/arm.md (arm_addsi3): Add alternative for addition of >> general register with general register or ARM constant into SP >> register. >> >> >> *** gcc/testsuite/ChangeLog *** >> >> 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * gcc.target/arm/empty_fiq_handler.c: New test. >> >> >> Testing: bootstrapped on ARMv7-A ARM mode & testsuite shows no regression. >> >> Is this ok for trunk? >> > > I see that "r" does not include the stack pointer (STACK_REG is separate > from GENERAL_REGs) so we are indeed missing > that constraint. > > Ok for trunk. > Thanks, > Kyrill > >> Best regards, >> >> Thomas > > Hi Thomas, The new test fails when compiled with -mthumb -march=armv5t: gcc.target/arm/empty_fiq_handler.c: In function 'fiq_handler': gcc.target/arm/empty_fiq_handler.c:11:1: error: interrupt Service Routines cannot be coded in Thumb mode Christophe
Hi Christophe, On 17/11/16 13:36, Christophe Lyon wrote: > On 16 November 2016 at 10:39, Kyrill Tkachov > <kyrylo.tkachov@foss.arm.com> wrote: >> >> On 09/11/16 16:19, Thomas Preudhomme wrote: >>> >>> Hi, >>> >>> This patch fixes the following ICE when building when compiling an empty >>> FIQ interrupt handler in ARM mode: >>> >>> empty_fiq_handler.c:5:1: error: insn does not satisfy its constraints: >>> } >>> ^ >>> >>> (insn/f 13 12 14 (set (reg/f:SI 13 sp) >>> (plus:SI (reg/f:SI 11 fp) >>> (const_int 4 [0x4]))) irq.c:5 4 {*arm_addsi3} >>> (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:SI 13 sp) >>> (plus:SI (reg/f:SI 11 fp) >>> (const_int 4 [0x4]))) >>> (nil))) >>> >>> The ICE was provoked by missing an alternative to reflect that ARM mode >>> can do an add of general register into sp which is unpredictable in Thumb >>> mode add immediate. >>> >>> ChangeLog entries are as follow: >>> >>> *** gcc/ChangeLog *** >>> >>> 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> >>> >>> * config/arm/arm.md (arm_addsi3): Add alternative for addition of >>> general register with general register or ARM constant into SP >>> register. >>> >>> >>> *** gcc/testsuite/ChangeLog *** >>> >>> 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> >>> >>> * gcc.target/arm/empty_fiq_handler.c: New test. >>> >>> >>> Testing: bootstrapped on ARMv7-A ARM mode & testsuite shows no regression. >>> >>> Is this ok for trunk? >>> >> >> I see that "r" does not include the stack pointer (STACK_REG is separate >> from GENERAL_REGs) so we are indeed missing >> that constraint. >> >> Ok for trunk. >> Thanks, >> Kyrill >> >>> Best regards, >>> >>> Thomas >> >> > > Hi Thomas, > > The new test fails when compiled with -mthumb -march=armv5t: > gcc.target/arm/empty_fiq_handler.c: In function 'fiq_handler': > gcc.target/arm/empty_fiq_handler.c:11:1: error: interrupt Service > Routines cannot be coded in Thumb mode Right, interrupt handler can only be compiled in the mode where the CPU boots. So for non Thumb-only targets it should be compiled with -marm. I'll push a patch tomorrow. Best regards, Thomas
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 8393f65bcf4c9c3e61b91e5adcd5f59ff7c6ec3f..70cd31f6cb176fe29efc1fbbf692bfc270ef5a1b 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -609,9 +609,9 @@ ;; (plus (reg rN) (reg sp)) into (reg rN). In this case reload will ;; put the duplicated register first, and not try the commutative version. (define_insn_and_split "*arm_addsi3" - [(set (match_operand:SI 0 "s_register_operand" "=rk,l,l ,l ,r ,k ,r,r ,k ,r ,k,k,r ,k ,r") - (plus:SI (match_operand:SI 1 "s_register_operand" "%0 ,l,0 ,l ,rk,k ,r,rk,k ,rk,k,r,rk,k ,rk") - (match_operand:SI 2 "reg_or_int_operand" "rk ,l,Py,Pd,rI,rI,k,Pj,Pj,L ,L,L,PJ,PJ,?n")))] + [(set (match_operand:SI 0 "s_register_operand" "=rk,l,l ,l ,r ,k ,r,k ,r ,k ,r ,k,k,r ,k ,r") + (plus:SI (match_operand:SI 1 "s_register_operand" "%0 ,l,0 ,l ,rk,k ,r,r ,rk,k ,rk,k,r,rk,k ,rk") + (match_operand:SI 2 "reg_or_int_operand" "rk ,l,Py,Pd,rI,rI,k,rI,Pj,Pj,L ,L,L,PJ,PJ,?n")))] "TARGET_32BIT" "@ add%?\\t%0, %0, %2 @@ -621,6 +621,7 @@ add%?\\t%0, %1, %2 add%?\\t%0, %1, %2 add%?\\t%0, %2, %1 + add%?\\t%0, %1, %2 addw%?\\t%0, %1, %2 addw%?\\t%0, %1, %2 sub%?\\t%0, %1, #%n2 @@ -640,10 +641,10 @@ operands[1], 0); DONE; " - [(set_attr "length" "2,4,4,4,4,4,4,4,4,4,4,4,4,4,16") + [(set_attr "length" "2,4,4,4,4,4,4,4,4,4,4,4,4,4,4,16") (set_attr "predicable" "yes") - (set_attr "predicable_short_it" "yes,yes,yes,yes,no,no,no,no,no,no,no,no,no,no,no") - (set_attr "arch" "t2,t2,t2,t2,*,*,*,t2,t2,*,*,a,t2,t2,*") + (set_attr "predicable_short_it" "yes,yes,yes,yes,no,no,no,no,no,no,no,no,no,no,no,no") + (set_attr "arch" "t2,t2,t2,t2,*,*,*,a,t2,t2,*,*,a,t2,t2,*") (set (attr "type") (if_then_else (match_operand 2 "const_int_operand" "") (const_string "alu_imm") (const_string "alu_sreg"))) diff --git a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c new file mode 100644 index 0000000000000000000000000000000000000000..bbcfd0e32f9d0cc60c8a013fd1bb584b21aaad16 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ + +/* Below code used to trigger an ICE due to missing constraints for + sp = fp + cst pattern. */ + +void fiq_handler (void) __attribute__((interrupt ("FIQ"))); + +void +fiq_handler (void) +{ +}
Hi, This patch fixes the following ICE when building when compiling an empty FIQ interrupt handler in ARM mode: empty_fiq_handler.c:5:1: error: insn does not satisfy its constraints: } ^ (insn/f 13 12 14 (set (reg/f:SI 13 sp) (plus:SI (reg/f:SI 11 fp) (const_int 4 [0x4]))) irq.c:5 4 {*arm_addsi3} (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:SI 13 sp) (plus:SI (reg/f:SI 11 fp) (const_int 4 [0x4]))) (nil))) The ICE was provoked by missing an alternative to reflect that ARM mode can do an add of general register into sp which is unpredictable in Thumb mode add immediate. ChangeLog entries are as follow: *** gcc/ChangeLog *** 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> * config/arm/arm.md (arm_addsi3): Add alternative for addition of general register with general register or ARM constant into SP register. *** gcc/testsuite/ChangeLog *** 2016-11-04 Thomas Preud'homme <thomas.preudhomme@arm.com> * gcc.target/arm/empty_fiq_handler.c: New test. Testing: bootstrapped on ARMv7-A ARM mode & testsuite shows no regression. Is this ok for trunk? Best regards, Thomas