| Message ID | 595d0543-5579-7cf7-06b5-032266c1dae0@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 06/12/16 11:36, Thomas Preudhomme wrote: > Ping? > > Best regards, > Ok if bootstrap and testing on those branches doesn't reveal any issues. Thanks, Kyrill > Thomas > > On 30/11/16 10:20, Thomas Preudhomme wrote: >> Hi, >> >> Is this ok to backport this fix together with its follow-up testcase fix to >> gcc-5-branch and gcc-6-branch? Both patches apply cleanly (patches attached for >> reference). >> >> >> 2016-11-30 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> Backport from mainline >> 2016-11-16 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> gcc/ >> * config/arm/arm.md (arm_addsi3): Add alternative for addition of >> general register with general register or ARM constant into SP >> register. >> >> gcc/testsuite/ >> * gcc.target/arm/empty_fiq_handler.c: New test. >> >> Backport from mainline >> 2016-11-21 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> gcc/testsuite/ >> * gcc.target/arm/empty_fiq_handler.c: Skip if -mthumb is passed in and >> target is Thumb-only. >> >> >> Best regards, >> >> Thomas >> >> >> On 16/11/16 09:39, Kyrill Tkachov 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 >>>
On 06/12/16 11:37, Kyrill Tkachov wrote: > > On 06/12/16 11:36, Thomas Preudhomme wrote: >> Ping? >> >> Best regards, >> > > Ok if bootstrap and testing on those branches doesn't reveal any issues. Both backport bootstrapped fine when configured with: --with-arch=armv7-a --with-mode=arm -with-fpu=neon-vfpv4 --with-float=hard --enable-languages=c,c++,fortran Testsuite did not show any regression when compared without the patch, hence both committed. Thanks! Thomas
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 47171b99682207226aa4f9a76d4dfb54d6c2814b..86df1c0366be6c4b9b4ebf76821a8100c4e9fc16 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -575,9 +575,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 @@ -587,6 +587,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 @@ -606,10 +607,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..8313f2199122be153a737946e817a5e3bee60372 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-skip-if "" { ! arm_cortex_m } { "-mthumb" } } */ + +/* 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) +{ +}