| Message ID | 50564bc7-7f41-f41e-b50f-9fb9589cdca7@foss.arm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 21 November 2016 at 12:03, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > On 17/11/16 20:04, Thomas Preudhomme wrote: >> >> 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. > > > I've committed the following patch as obvious: > > Interrupt handlers on ARM can only be compiled in the execution mode where > the processor boot. That is -mthumb for Thumb-only devices, -marm otherwise. > This changes the empty_fiq_handler to skip the test when -mthumb is passed > but the processor boot in ARM mode. > > ChangeLog entry is as follows: > > *** gcc/testsuite/ChangeLog *** > > 2016-11-17 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * gcc.target/arm/empty_fiq_handler.c: Skip if -mthumb is passed in > and > target is Thumb-only. > Hi Thomas, As for another patch of yours, this doesn't work in GCC was configured --with-mode=thumb (the test is not skipped). But I guess you know that already :-) Christophe > Best regards, > > Thomas
On 08/12/16 08:46, Christophe Lyon wrote: > On 21 November 2016 at 12:03, Thomas Preudhomme > <thomas.preudhomme@foss.arm.com> wrote: >> On 17/11/16 20:04, Thomas Preudhomme wrote: >>> >>> 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. >> >> >> I've committed the following patch as obvious: >> >> Interrupt handlers on ARM can only be compiled in the execution mode where >> the processor boot. That is -mthumb for Thumb-only devices, -marm otherwise. >> This changes the empty_fiq_handler to skip the test when -mthumb is passed >> but the processor boot in ARM mode. >> >> ChangeLog entry is as follows: >> >> *** gcc/testsuite/ChangeLog *** >> >> 2016-11-17 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * gcc.target/arm/empty_fiq_handler.c: Skip if -mthumb is passed in >> and >> target is Thumb-only. >> > > Hi Thomas, > > As for another patch of yours, this doesn't work in GCC was configured > --with-mode=thumb > (the test is not skipped). But I guess you know that already :-) Oh no! I wasn't aware this patch also had the issue on trunk. Ok, I better get going with adding a dejagnu procedure to check --with-mode. Thanks for the feedback and sorry for that. Best regards, Thomas
On 8 December 2016 at 10:09, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > > > On 08/12/16 08:46, Christophe Lyon wrote: >> >> On 21 November 2016 at 12:03, Thomas Preudhomme >> <thomas.preudhomme@foss.arm.com> wrote: >>> >>> On 17/11/16 20:04, Thomas Preudhomme wrote: >>>> >>>> >>>> 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. >>> >>> >>> >>> I've committed the following patch as obvious: >>> >>> Interrupt handlers on ARM can only be compiled in the execution mode >>> where >>> the processor boot. That is -mthumb for Thumb-only devices, -marm >>> otherwise. >>> This changes the empty_fiq_handler to skip the test when -mthumb is >>> passed >>> but the processor boot in ARM mode. >>> >>> ChangeLog entry is as follows: >>> >>> *** gcc/testsuite/ChangeLog *** >>> >>> 2016-11-17 Thomas Preud'homme <thomas.preudhomme@arm.com> >>> >>> * gcc.target/arm/empty_fiq_handler.c: Skip if -mthumb is passed >>> in >>> and >>> target is Thumb-only. >>> >> >> Hi Thomas, >> >> As for another patch of yours, this doesn't work in GCC was configured >> --with-mode=thumb >> (the test is not skipped). But I guess you know that already :-) > > > Oh no! I wasn't aware this patch also had the issue on trunk. Ok, I better > get going with adding a dejagnu procedure to check --with-mode. > > Thanks for the feedback and sorry for that. > No problem, I know it's a nightmare to cover all the combinations :( > Best regards, > > Thomas
diff --git a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c index bbcfd0e32f9d0cc60c8a013fd1bb584b21aaad16..8313f2199122be153a737946e817a5e3bee60372 100644 --- a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c +++ b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c @@ -1,4 +1,5 @@ /* { 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. */