| Message ID | BANLkTin+xYT+puPkdOHUuR5K3E+xUHH0Lw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 08/04/11 08:36, Carrot Wei wrote: > Hi > > This patch moves the length computation of push_multi into a C > function to avoid the usage of GNU extension. Please put the comment regarding the calculation of the length attribute along with the pattern in thumb2.md as well. > > Tested on qemu without regression. OK to install? Ok with that change. Thanks for fixing this up. cheers Ramana > > thanks > Carrot > > ChangeLog: > 2011-04-08 Wei Guozhi<carrot@google.com> > > PR target/47855 > * config/arm/arm-protos.h (arm_attr_length_push_multi): New prototype. > * config/arm/arm.c (arm_attr_length_push_multi): New function. > * config/arm/arm.md (*push_multi): Change the length computation to > call a C function. > > > Index: arm.c > =================================================================== > --- arm.c (revision 172158) > +++ arm.c (working copy) > @@ -23696,4 +23696,30 @@ arm_preferred_rename_class (reg_class_t > return NO_REGS; > } > > +/* Compute the atrribute "length" of insn "*push_multi". > + So this function MUST be kept in sync with that insn pattern. */ > +int > +arm_attr_length_push_multi(rtx parallel_op, rtx first_op) > +{ > + int i, regno, hi_reg; > + int num_saves = XVECLEN (parallel_op, 0); > + > + /* ARM mode. */ > + if (TARGET_ARM) > + return 4; > + > + /* Thumb2 mode. */ > + regno = REGNO (first_op); > + hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS)&& (regno != LR_REGNUM); > + for (i = 1; i< num_saves&& !hi_reg; i++) > + { > + regno = REGNO (XEXP (XVECEXP (parallel_op, 0, i), 0)); > + hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS)&& (regno != LR_REGNUM); > + } > + > + if (!hi_reg) > + return 2; > + return 4; > +} > + > #include "gt-arm.h" > Index: arm-protos.h > =================================================================== > --- arm-protos.h (revision 172158) > +++ arm-protos.h (working copy) > @@ -152,6 +152,7 @@ extern void arm_expand_sync (enum machin > extern const char *arm_output_memory_barrier (rtx *); > extern const char *arm_output_sync_insn (rtx, rtx *); > extern unsigned int arm_sync_loop_insns (rtx , rtx *); > +extern int arm_attr_length_push_multi(rtx, rtx); > > #if defined TREE_CODE > extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree); > Index: arm.md > =================================================================== > --- arm.md (revision 172158) > +++ arm.md (working copy) > @@ -10290,27 +10290,7 @@ > }" > [(set_attr "type" "store4") > (set (attr "length") > - (if_then_else > - (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0)) > - (ne (symbol_ref "{ > - /* Check if there are any high register (except lr) > - references in the list. KEEP the following iteration > - in sync with the template above. */ > - int i, regno, hi_reg; > - int num_saves = XVECLEN (operands[2], 0); > - regno = REGNO (operands[1]); > - hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS) > - && (regno != LR_REGNUM); > - for (i = 1; i< num_saves&& !hi_reg; i++) > - { > - regno = REGNO (XEXP (XVECEXP (operands[2], 0, i), 0)); > - hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS) > - && (regno != LR_REGNUM); > - } > - !hi_reg; }") > - (const_int 0))) > - (const_int 2) > - (const_int 4)))] > + (symbol_ref "arm_attr_length_push_multi (operands[2], operands[1])"))] > ) > > (define_insn "stack_tie" > > On Thu, Apr 7, 2011 at 7:30 PM, Ramana Radhakrishnan > <ramana.radhakrishnan@linaro.org> wrote: >> On 07/04/11 12:08, Carrot Wei wrote: >>> >>> On Thu, Apr 7, 2011 at 5:31 PM, Richard Sandiford >>> <richard.sandiford@linaro.org> wrote: >>>> >>>> Hi Carrot, >>>> >>>> Sorry if this has already been reported, but the patch breaks bootstrap >>>> of arm-linux-gnueabi (or cross builds with --enable-werror). The problem >>>> is that this... >>>> >>>> uses a statement expression -- i.e. ({ code; result; }) -- which is >>>> a GNU extension. >>>> >>>> I suppose a simple fix would be to put the code into an arm.c function. >>>> >>> >>> Thank you for the report, I will add this fix to the second part of the >>> patch. >> >> It would be worth unbreaking bootstrap as a separate patch and not >> conflating it with a different set of improvements. >> >> Ramana >> >>> >>> Carrot >> >>
Sorry, which pattern in thumb2.md? thanks Carrot On Fri, Apr 8, 2011 at 4:35 PM, Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org> wrote: > On 08/04/11 08:36, Carrot Wei wrote: >> >> Hi >> >> This patch moves the length computation of push_multi into a C >> function to avoid the usage of GNU extension. > > Please put the comment regarding the calculation of the length attribute > along with the pattern in thumb2.md as well. > >> >> Tested on qemu without regression. OK to install? > > Ok with that change. > > Thanks for fixing this up. > > cheers > Ramana >> >> thanks >> Carrot >> >> ChangeLog: >> 2011-04-08 Wei Guozhi<carrot@google.com> >> >> PR target/47855 >> * config/arm/arm-protos.h (arm_attr_length_push_multi): New >> prototype. >> * config/arm/arm.c (arm_attr_length_push_multi): New function. >> * config/arm/arm.md (*push_multi): Change the length computation >> to >> call a C function. >> >> >> Index: arm.c >> =================================================================== >> --- arm.c (revision 172158) >> +++ arm.c (working copy) >> @@ -23696,4 +23696,30 @@ arm_preferred_rename_class (reg_class_t >> return NO_REGS; >> } >> >> +/* Compute the atrribute "length" of insn "*push_multi". >> + So this function MUST be kept in sync with that insn pattern. */ >> +int >> +arm_attr_length_push_multi(rtx parallel_op, rtx first_op) >> +{ >> + int i, regno, hi_reg; >> + int num_saves = XVECLEN (parallel_op, 0); >> + >> + /* ARM mode. */ >> + if (TARGET_ARM) >> + return 4; >> + >> + /* Thumb2 mode. */ >> + regno = REGNO (first_op); >> + hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS)&& (regno != LR_REGNUM); >> + for (i = 1; i< num_saves&& !hi_reg; i++) >> + { >> + regno = REGNO (XEXP (XVECEXP (parallel_op, 0, i), 0)); >> + hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS)&& (regno != >> LR_REGNUM); >> + } >> + >> + if (!hi_reg) >> + return 2; >> + return 4; >> +} >> + >> #include "gt-arm.h" >> Index: arm-protos.h >> =================================================================== >> --- arm-protos.h (revision 172158) >> +++ arm-protos.h (working copy) >> @@ -152,6 +152,7 @@ extern void arm_expand_sync (enum machin >> extern const char *arm_output_memory_barrier (rtx *); >> extern const char *arm_output_sync_insn (rtx, rtx *); >> extern unsigned int arm_sync_loop_insns (rtx , rtx *); >> +extern int arm_attr_length_push_multi(rtx, rtx); >> >> #if defined TREE_CODE >> extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, >> tree); >> Index: arm.md >> =================================================================== >> --- arm.md (revision 172158) >> +++ arm.md (working copy) >> @@ -10290,27 +10290,7 @@ >> }" >> [(set_attr "type" "store4") >> (set (attr "length") >> - (if_then_else >> - (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0)) >> - (ne (symbol_ref "{ >> - /* Check if there are any high register (except lr) >> - references in the list. KEEP the following >> iteration >> - in sync with the template above. */ >> - int i, regno, hi_reg; >> - int num_saves = XVECLEN (operands[2], 0); >> - regno = REGNO (operands[1]); >> - hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS) >> - && (regno != LR_REGNUM); >> - for (i = 1; i< num_saves&& !hi_reg; i++) >> - { >> - regno = REGNO (XEXP (XVECEXP (operands[2], 0, i), >> 0)); >> - hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS) >> - && (regno != LR_REGNUM); >> - } >> - !hi_reg; }") >> - (const_int 0))) >> - (const_int 2) >> - (const_int 4)))] >> + (symbol_ref "arm_attr_length_push_multi (operands[2], >> operands[1])"))] >> ) >> >> (define_insn "stack_tie" >> >> On Thu, Apr 7, 2011 at 7:30 PM, Ramana Radhakrishnan >> <ramana.radhakrishnan@linaro.org> wrote: >>> >>> On 07/04/11 12:08, Carrot Wei wrote: >>>> >>>> On Thu, Apr 7, 2011 at 5:31 PM, Richard Sandiford >>>> <richard.sandiford@linaro.org> wrote: >>>>> >>>>> Hi Carrot, >>>>> >>>>> Sorry if this has already been reported, but the patch breaks bootstrap >>>>> of arm-linux-gnueabi (or cross builds with --enable-werror). The >>>>> problem >>>>> is that this... >>>>> >>>>> uses a statement expression -- i.e. ({ code; result; }) -- which is >>>>> a GNU extension. >>>>> >>>>> I suppose a simple fix would be to put the code into an arm.c function. >>>>> >>>> >>>> Thank you for the report, I will add this fix to the second part of the >>>> patch. >>> >>> It would be worth unbreaking bootstrap as a separate patch and not >>> conflating it with a different set of improvements. >>> >>> Ramana >>> >>>> >>>> Carrot >>> >>> > >
On 8 April 2011 09:52, Carrot Wei <carrot@google.com> wrote: > Sorry, which pattern in thumb2.md? Sorry, I meant to say push_multi in arm.md . Not enough coffee yet this morning. cheers Ramana > > thanks > Carrot > > On Fri, Apr 8, 2011 at 4:35 PM, Ramana Radhakrishnan > <ramana.radhakrishnan@linaro.org> wrote: >> On 08/04/11 08:36, Carrot Wei wrote: >>> >>> Hi >>> >>> This patch moves the length computation of push_multi into a C >>> function to avoid the usage of GNU extension. >> >> Please put the comment regarding the calculation of the length attribute >> along with the pattern in thumb2.md as well. >> >>> >>> Tested on qemu without regression. OK to install? >> >> Ok with that change. >> >> Thanks for fixing this up. >> >> cheers >> Ramana >>> >>> thanks >>> Carrot >>> >>> ChangeLog: >>> 2011-04-08 Wei Guozhi<carrot@google.com> >>> >>> PR target/47855 >>> * config/arm/arm-protos.h (arm_attr_length_push_multi): New >>> prototype. >>> * config/arm/arm.c (arm_attr_length_push_multi): New function. >>> * config/arm/arm.md (*push_multi): Change the length computation >>> to >>> call a C function. >>> >>> >>> Index: arm.c >>> =================================================================== >>> --- arm.c (revision 172158) >>> +++ arm.c (working copy) >>> @@ -23696,4 +23696,30 @@ arm_preferred_rename_class (reg_class_t >>> return NO_REGS; >>> } >>> >>> +/* Compute the atrribute "length" of insn "*push_multi". >>> + So this function MUST be kept in sync with that insn pattern. */ >>> +int >>> +arm_attr_length_push_multi(rtx parallel_op, rtx first_op) >>> +{ >>> + int i, regno, hi_reg; >>> + int num_saves = XVECLEN (parallel_op, 0); >>> + >>> + /* ARM mode. */ >>> + if (TARGET_ARM) >>> + return 4; >>> + >>> + /* Thumb2 mode. */ >>> + regno = REGNO (first_op); >>> + hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS)&& (regno != LR_REGNUM); >>> + for (i = 1; i< num_saves&& !hi_reg; i++) >>> + { >>> + regno = REGNO (XEXP (XVECEXP (parallel_op, 0, i), 0)); >>> + hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS)&& (regno != >>> LR_REGNUM); >>> + } >>> + >>> + if (!hi_reg) >>> + return 2; >>> + return 4; >>> +} >>> + >>> #include "gt-arm.h" >>> Index: arm-protos.h >>> =================================================================== >>> --- arm-protos.h (revision 172158) >>> +++ arm-protos.h (working copy) >>> @@ -152,6 +152,7 @@ extern void arm_expand_sync (enum machin >>> extern const char *arm_output_memory_barrier (rtx *); >>> extern const char *arm_output_sync_insn (rtx, rtx *); >>> extern unsigned int arm_sync_loop_insns (rtx , rtx *); >>> +extern int arm_attr_length_push_multi(rtx, rtx); >>> >>> #if defined TREE_CODE >>> extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, >>> tree); >>> Index: arm.md >>> =================================================================== >>> --- arm.md (revision 172158) >>> +++ arm.md (working copy) >>> @@ -10290,27 +10290,7 @@ >>> }" >>> [(set_attr "type" "store4") >>> (set (attr "length") >>> - (if_then_else >>> - (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0)) >>> - (ne (symbol_ref "{ >>> - /* Check if there are any high register (except lr) >>> - references in the list. KEEP the following >>> iteration >>> - in sync with the template above. */ >>> - int i, regno, hi_reg; >>> - int num_saves = XVECLEN (operands[2], 0); >>> - regno = REGNO (operands[1]); >>> - hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS) >>> - && (regno != LR_REGNUM); >>> - for (i = 1; i< num_saves&& !hi_reg; i++) >>> - { >>> - regno = REGNO (XEXP (XVECEXP (operands[2], 0, i), >>> 0)); >>> - hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS) >>> - && (regno != LR_REGNUM); >>> - } >>> - !hi_reg; }") >>> - (const_int 0))) >>> - (const_int 2) >>> - (const_int 4)))] >>> + (symbol_ref "arm_attr_length_push_multi (operands[2], >>> operands[1])"))] >>> ) >>> >>> (define_insn "stack_tie" >>> >>> On Thu, Apr 7, 2011 at 7:30 PM, Ramana Radhakrishnan >>> <ramana.radhakrishnan@linaro.org> wrote: >>>> >>>> On 07/04/11 12:08, Carrot Wei wrote: >>>>> >>>>> On Thu, Apr 7, 2011 at 5:31 PM, Richard Sandiford >>>>> <richard.sandiford@linaro.org> wrote: >>>>>> >>>>>> Hi Carrot, >>>>>> >>>>>> Sorry if this has already been reported, but the patch breaks bootstrap >>>>>> of arm-linux-gnueabi (or cross builds with --enable-werror). The >>>>>> problem >>>>>> is that this... >>>>>> >>>>>> uses a statement expression -- i.e. ({ code; result; }) -- which is >>>>>> a GNU extension. >>>>>> >>>>>> I suppose a simple fix would be to put the code into an arm.c function. >>>>>> >>>>> >>>>> Thank you for the report, I will add this fix to the second part of the >>>>> patch. >>>> >>>> It would be worth unbreaking bootstrap as a separate patch and not >>>> conflating it with a different set of improvements. >>>> >>>> Ramana >>>> >>>>> >>>>> Carrot >>>> >>>> >> >> >
Index: arm.c =================================================================== --- arm.c (revision 172158) +++ arm.c (working copy) @@ -23696,4 +23696,30 @@ arm_preferred_rename_class (reg_class_t return NO_REGS; } +/* Compute the atrribute "length" of insn "*push_multi". + So this function MUST be kept in sync with that insn pattern. */ +int +arm_attr_length_push_multi(rtx parallel_op, rtx first_op) +{ + int i, regno, hi_reg; + int num_saves = XVECLEN (parallel_op, 0); + + /* ARM mode. */ + if (TARGET_ARM) + return 4; + + /* Thumb2 mode. */ + regno = REGNO (first_op); + hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS) && (regno != LR_REGNUM); + for (i = 1; i < num_saves && !hi_reg; i++) + { + regno = REGNO (XEXP (XVECEXP (parallel_op, 0, i), 0)); + hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS) && (regno != LR_REGNUM); + } + + if (!hi_reg) + return 2; + return 4; +} + #include "gt-arm.h" Index: arm-protos.h =================================================================== --- arm-protos.h (revision 172158) +++ arm-protos.h (working copy) @@ -152,6 +152,7 @@ extern void arm_expand_sync (enum machin extern const char *arm_output_memory_barrier (rtx *); extern const char *arm_output_sync_insn (rtx, rtx *); extern unsigned int arm_sync_loop_insns (rtx , rtx *); +extern int arm_attr_length_push_multi(rtx, rtx); #if defined TREE_CODE extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree); Index: arm.md =================================================================== --- arm.md (revision 172158) +++ arm.md (working copy) @@ -10290,27 +10290,7 @@ }" [(set_attr "type" "store4") (set (attr "length") - (if_then_else - (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0)) - (ne (symbol_ref "{ - /* Check if there are any high register (except lr) - references in the list. KEEP the following iteration - in sync with the template above. */ - int i, regno, hi_reg; - int num_saves = XVECLEN (operands[2], 0); - regno = REGNO (operands[1]); - hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS) - && (regno != LR_REGNUM); - for (i = 1; i < num_saves && !hi_reg; i++) - { - regno = REGNO (XEXP (XVECEXP (operands[2], 0, i), 0)); - hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS) - && (regno != LR_REGNUM); - } - !hi_reg; }") - (const_int 0))) - (const_int 2) - (const_int 4)))] + (symbol_ref "arm_attr_length_push_multi (operands[2], operands[1])"))] ) (define_insn "stack_tie"
Hi This patch moves the length computation of push_multi into a C function to avoid the usage of GNU extension. Tested on qemu without regression. OK to install? thanks Carrot ChangeLog: 2011-04-08 Wei Guozhi <carrot@google.com> PR target/47855 * config/arm/arm-protos.h (arm_attr_length_push_multi): New prototype. * config/arm/arm.c (arm_attr_length_push_multi): New function. * config/arm/arm.md (*push_multi): Change the length computation to call a C function. On Thu, Apr 7, 2011 at 7:30 PM, Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org> wrote: > On 07/04/11 12:08, Carrot Wei wrote: >> >> On Thu, Apr 7, 2011 at 5:31 PM, Richard Sandiford >> <richard.sandiford@linaro.org> wrote: >>> >>> Hi Carrot, >>> >>> Sorry if this has already been reported, but the patch breaks bootstrap >>> of arm-linux-gnueabi (or cross builds with --enable-werror). The problem >>> is that this... >>> >>> uses a statement expression -- i.e. ({ code; result; }) -- which is >>> a GNU extension. >>> >>> I suppose a simple fix would be to put the code into an arm.c function. >>> >> >> Thank you for the report, I will add this fix to the second part of the >> patch. > > It would be worth unbreaking bootstrap as a separate patch and not > conflating it with a different set of improvements. > > Ramana > >> >> Carrot > >