* [PATCH, ARM 6/6] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline @ 2015-12-17 8:18 Thomas Preud'homme 2016-05-17 10:14 ` [PATCH, ARM 6/7, ping1] " Thomas Preudhomme 0 siblings, 1 reply; 7+ messages in thread From: Thomas Preud'homme @ 2015-12-17 8:18 UTC (permalink / raw) To: gcc-patches, Richard Earnshaw, Ramana Radhakrishnan, Kyrylo Tkachov Hi, This patch is part of a patch series to add support for ARMv8-M[1] to GCC. This specific patch makes the compiler start generating code with the new CB(N)Z and (U|S)DIV instructions for ARMv8-M Baseline. Sharing of instruction patterns for div insn template with ARM or Thumb-2 was done by allowing %? punctuation character for Thumb-1. This is safe to do since the compiler would fault in arm_print_condition if a condition code is not handled by a branch in Thumb1. Unfortunately, cbz cannot be shared with cbranchsi4 because it would lead to worse code for Thumb-1. Indeed, choosing cb(n)z over the other alternatives for cbranchsi4 depends on the distance between target and pc which lead insn-attrtab to evaluate the minimum length of this pattern to be 2 as it cannot computer the distance statically. It would be possible to determine that this alternative is not available for non ARMv8-M Thumb-1 target statically but genattrtab is not currently capable to do it, so this is for a later patch. [1] For a quick overview of ARMv8-M please refer to the initial cover letter. ChangeLog entry is as follows: *** gcc/ChangeLog *** 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? valid for Thumb-1. * config/arm/arm.h (TARGET_HAVE_CBZ): Define. (TARGET_IDIV): Set for all Thumb targets provided they have hardware divide feature. * config/arm/thumb1.md (thumb1_cbz): New insn. diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 015df50..247f144 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -263,9 +263,12 @@ extern void (*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip provides the movw and movt instructions. */ #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) +/* Nonzero if this chip provides the cb{n}z instruction. */ +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) + /* Nonzero if integer division instructions supported. */ #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) /* Nonzero if disallow volatile memory access in IT block. */ #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index d832309..5ef3a1d 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -22568,7 +22568,7 @@ arm_print_operand_punct_valid_p (unsigned char code) { return (code == '@' || code == '|' || code == '.' || code == '(' || code == ')' || code == '#' - || (TARGET_32BIT && (code == '?')) + || code == '?' || (TARGET_THUMB2 && (code == '!')) || (TARGET_THUMB && (code == '_'))); } diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index 7e3bcb4..074b267 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -973,6 +973,92 @@ DONE; }) +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline profile, +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads to +;; code generation difference for ARMv6-M because the minimum length of the +;; instruction becomes 2 even for it due to a limitation in genattrtab's +;; handling of pc in the length condition. +(define_insn "thumb1_cbz" + [(set (pc) (if_then_else + (match_operator 0 "equality_operator" + [(match_operand:SI 1 "s_register_operand" "l") + (const_int 0)]) + (label_ref (match_operand 2 "" "")) + (pc)))] + "TARGET_THUMB1 && TARGET_HAVE_MOVT" +{ + if (get_attr_length (insn) == 2) + { + if (GET_CODE (operands[0]) == EQ) + return "cbz\t%1, %l2"; + else + return "cbnz\t%1, %l2"; + } + else + { + rtx t = cfun->machine->thumb1_cc_insn; + if (t != NULL_RTX) + { + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) + t = NULL_RTX; + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) + { + if (!noov_comparison_operator (operands[0], VOIDmode)) + t = NULL_RTX; + } + else if (cfun->machine->thumb1_cc_mode != CCmode) + t = NULL_RTX; + } + if (t == NULL_RTX) + { + output_asm_insn ("cmp\t%1, #0", operands); + cfun->machine->thumb1_cc_insn = insn; + cfun->machine->thumb1_cc_op0 = operands[1]; + cfun->machine->thumb1_cc_op1 = operands[2]; + cfun->machine->thumb1_cc_mode = CCmode; + } + else + /* Ensure we emit the right type of condition code on the jump. */ + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, + CC_REGNUM); + + switch (get_attr_length (insn)) + { + case 4: return "b%d0\t%l2"; + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; + default: gcc_unreachable (); + } + } +} + [(set (attr "far_jump") + (if_then_else + (eq_attr "length" "8") + (const_string "yes") + (const_string "no"))) + (set (attr "length") + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) + (le (minus (match_dup 2) (pc)) (const_int 128)) + (not (match_test "which_alternative"))) + (const_int 2) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) + (le (minus (match_dup 2) (pc)) (const_int 256))) + (const_int 4) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) + (le (minus (match_dup 2) (pc)) (const_int 2048))) + (const_int 6) + (const_int 8))))) + (set (attr "type") + (if_then_else + (eq_attr "length" "2") + (const_string "branch") + (const_string "multiple")))] +) + (define_insn "cbranchsi4_insn" [(set (pc) (if_then_else (match_operator 0 "arm_comparison_operator" Testing: * Toolchain was built successfully with and without the ARMv8-M support patches with the following multilib list: armv6-m,armv7-m,armv7e-m,cortex-m7. The code generation for crtbegin.o, crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a, libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a, libstdc++.a and libsupc++.a is unchanged for all these targets. * GCC also showed no testsuite regression when targeting ARMv8-M Baseline compared to ARMv6-M on ARM Fast Models and when targeting ARMv6-M and ARMv7-M (compared to without the patch) * GCC was bootstrapped successfully targeting Thumb-1 and targeting Thumb-2 Is this ok for stage3? Best regards, Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline 2015-12-17 8:18 [PATCH, ARM 6/6] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline Thomas Preud'homme @ 2016-05-17 10:14 ` Thomas Preudhomme 2016-05-20 13:23 ` Kyrill Tkachov 0 siblings, 1 reply; 7+ messages in thread From: Thomas Preudhomme @ 2016-05-17 10:14 UTC (permalink / raw) To: Richard Earnshaw, Ramana Radhakrishnan, Kyrylo Tkachov; +Cc: gcc-patches Ping? *** gcc/ChangeLog *** 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? valid for Thumb-1. * config/arm/arm.h (TARGET_HAVE_CBZ): Define. (TARGET_IDIV): Set for all Thumb targets provided they have hardware divide feature. * config/arm/thumb1.md (thumb1_cbz): New insn. diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index f42e996e5a7ce979fe406b8261d50fb2ba005f6b..347b5b0a5cc0bc1e3b5020c8124d968e76ce48a4 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook) (void); /* Nonzero if this chip provides the movw and movt instructions. */ #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) +/* Nonzero if this chip provides the cb{n}z instruction. */ +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) + /* Nonzero if integer division instructions supported. */ #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) /* Nonzero if disallow volatile memory access in IT block. */ #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 13b4b71ac8f9c1da8ef1945f7ff6985ca59f6832..445972ce0e3fd27d4411840ff69e9edbb23994fc 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -22684,7 +22684,7 @@ arm_print_operand_punct_valid_p (unsigned char code) { return (code == '@' || code == '|' || code == '.' || code == '(' || code == ')' || code == '#' - || (TARGET_32BIT && (code == '?')) + || code == '?' || (TARGET_THUMB2 && (code == '!')) || (TARGET_THUMB && (code == '_'))); } diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index 4572456b8bc98503061846cad94bc642943db3a2..1b01ef6ce731fe3ff37c3d8c048fb9d5e7829b35 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -973,6 +973,92 @@ DONE; }) +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline profile, +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads to +;; code generation difference for ARMv6-M because the minimum length of the +;; instruction becomes 2 even for it due to a limitation in genattrtab's +;; handling of pc in the length condition. +(define_insn "thumb1_cbz" + [(set (pc) (if_then_else + (match_operator 0 "equality_operator" + [(match_operand:SI 1 "s_register_operand" "l") + (const_int 0)]) + (label_ref (match_operand 2 "" "")) + (pc)))] + "TARGET_THUMB1 && TARGET_HAVE_MOVT" +{ + if (get_attr_length (insn) == 2) + { + if (GET_CODE (operands[0]) == EQ) + return "cbz\t%1, %l2"; + else + return "cbnz\t%1, %l2"; + } + else + { + rtx t = cfun->machine->thumb1_cc_insn; + if (t != NULL_RTX) + { + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) + t = NULL_RTX; + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) + { + if (!noov_comparison_operator (operands[0], VOIDmode)) + t = NULL_RTX; + } + else if (cfun->machine->thumb1_cc_mode != CCmode) + t = NULL_RTX; + } + if (t == NULL_RTX) + { + output_asm_insn ("cmp\t%1, #0", operands); + cfun->machine->thumb1_cc_insn = insn; + cfun->machine->thumb1_cc_op0 = operands[1]; + cfun->machine->thumb1_cc_op1 = operands[2]; + cfun->machine->thumb1_cc_mode = CCmode; + } + else + /* Ensure we emit the right type of condition code on the jump. */ + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, + CC_REGNUM); + + switch (get_attr_length (insn)) + { + case 4: return "b%d0\t%l2"; + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; + default: gcc_unreachable (); + } + } +} + [(set (attr "far_jump") + (if_then_else + (eq_attr "length" "8") + (const_string "yes") + (const_string "no"))) + (set (attr "length") + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) + (le (minus (match_dup 2) (pc)) (const_int 128)) + (not (match_test "which_alternative"))) + (const_int 2) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) + (le (minus (match_dup 2) (pc)) (const_int 256))) + (const_int 4) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) + (le (minus (match_dup 2) (pc)) (const_int 2048))) + (const_int 6) + (const_int 8))))) + (set (attr "type") + (if_then_else + (eq_attr "length" "2") + (const_string "branch") + (const_string "multiple")))] +) + (define_insn "cbranchsi4_insn" [(set (pc) (if_then_else (match_operator 0 "arm_comparison_operator" Best regards, Thomas On Thursday 17 December 2015 16:17:52 Thomas Preud'homme wrote: > Hi, > > This patch is part of a patch series to add support for ARMv8-M[1] to GCC. > This specific patch makes the compiler start generating code with the new > CB(N)Z and (U|S)DIV instructions for ARMv8-M Baseline. > > Sharing of instruction patterns for div insn template with ARM or Thumb-2 > was done by allowing %? punctuation character for Thumb-1. This is safe to > do since the compiler would fault in arm_print_condition if a condition > code is not handled by a branch in Thumb1. > > Unfortunately, cbz cannot be shared with cbranchsi4 because it would lead to > worse code for Thumb-1. Indeed, choosing cb(n)z over the other alternatives > for cbranchsi4 depends on the distance between target and pc which lead > insn-attrtab to evaluate the minimum length of this pattern to be 2 as it > cannot computer the distance statically. It would be possible to determine > that this alternative is not available for non ARMv8-M Thumb-1 target > statically but genattrtab is not currently capable to do it, so this is for > a later patch. > > > [1] For a quick overview of ARMv8-M please refer to the initial cover > letter. > > ChangeLog entry is as follows: > > > *** gcc/ChangeLog *** > > 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? valid > for Thumb-1. > * config/arm/arm.h (TARGET_HAVE_CBZ): Define. > (TARGET_IDIV): Set for all Thumb targets provided they have hardware > divide feature. > * config/arm/thumb1.md (thumb1_cbz): New insn. > > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > index 015df50..247f144 100644 > --- a/gcc/config/arm/arm.h > +++ b/gcc/config/arm/arm.h > @@ -263,9 +263,12 @@ extern void > (*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip > provides the movw and movt instructions. */ #define > TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) > > +/* Nonzero if this chip provides the cb{n}z instruction. */ > +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) > + > /* Nonzero if integer division instructions supported. */ > #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ > - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) > + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) > > /* Nonzero if disallow volatile memory access in IT block. */ > #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index d832309..5ef3a1d 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -22568,7 +22568,7 @@ arm_print_operand_punct_valid_p (unsigned char code) > { > return (code == '@' || code == '|' || code == '.' > > || code == '(' || code == ')' || code == '#' > > - || (TARGET_32BIT && (code == '?')) > + || code == '?' > > || (TARGET_THUMB2 && (code == '!')) > || (TARGET_THUMB && (code == '_'))); > > } > diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md > index 7e3bcb4..074b267 100644 > --- a/gcc/config/arm/thumb1.md > +++ b/gcc/config/arm/thumb1.md > @@ -973,6 +973,92 @@ > DONE; > }) > > +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline profile, > +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads > to +;; code generation difference for ARMv6-M because the minimum length of > the +;; instruction becomes 2 even for it due to a limitation in > genattrtab's +;; handling of pc in the length condition. > +(define_insn "thumb1_cbz" > + [(set (pc) (if_then_else > + (match_operator 0 "equality_operator" > + [(match_operand:SI 1 "s_register_operand" "l") > + (const_int 0)]) > + (label_ref (match_operand 2 "" "")) > + (pc)))] > + "TARGET_THUMB1 && TARGET_HAVE_MOVT" > +{ > + if (get_attr_length (insn) == 2) > + { > + if (GET_CODE (operands[0]) == EQ) > + return "cbz\t%1, %l2"; > + else > + return "cbnz\t%1, %l2"; > + } > + else > + { > + rtx t = cfun->machine->thumb1_cc_insn; > + if (t != NULL_RTX) > + { > + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) > + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) > + t = NULL_RTX; > + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) > + { > + if (!noov_comparison_operator (operands[0], VOIDmode)) > + t = NULL_RTX; > + } > + else if (cfun->machine->thumb1_cc_mode != CCmode) > + t = NULL_RTX; > + } > + if (t == NULL_RTX) > + { > + output_asm_insn ("cmp\t%1, #0", operands); > + cfun->machine->thumb1_cc_insn = insn; > + cfun->machine->thumb1_cc_op0 = operands[1]; > + cfun->machine->thumb1_cc_op1 = operands[2]; > + cfun->machine->thumb1_cc_mode = CCmode; > + } > + else > + /* Ensure we emit the right type of condition code on the jump. */ > + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, > + CC_REGNUM); > + > + switch (get_attr_length (insn)) > + { > + case 4: return "b%d0\t%l2"; > + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; > + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; > + default: gcc_unreachable (); > + } > + } > +} > + [(set (attr "far_jump") > + (if_then_else > + (eq_attr "length" "8") > + (const_string "yes") > + (const_string "no"))) > + (set (attr "length") > + (if_then_else > + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) > + (le (minus (match_dup 2) (pc)) (const_int 128)) > + (not (match_test "which_alternative"))) > + (const_int 2) > + (if_then_else > + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) > + (le (minus (match_dup 2) (pc)) (const_int 256))) > + (const_int 4) > + (if_then_else > + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) > + (le (minus (match_dup 2) (pc)) (const_int 2048))) > + (const_int 6) > + (const_int 8))))) > + (set (attr "type") > + (if_then_else > + (eq_attr "length" "2") > + (const_string "branch") > + (const_string "multiple")))] > +) > + > (define_insn "cbranchsi4_insn" > [(set (pc) (if_then_else > (match_operator 0 "arm_comparison_operator" > > > Testing: > > * Toolchain was built successfully with and without the ARMv8-M support > patches with the following multilib list: > armv6-m,armv7-m,armv7e-m,cortex-m7. The code generation for crtbegin.o, > crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a, > libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a, libstdc++.a > and libsupc++.a is unchanged for all these targets. > > * GCC also showed no testsuite regression when targeting ARMv8-M Baseline > compared to ARMv6-M on ARM Fast Models and when targeting ARMv6-M and > ARMv7-M (compared to without the patch) * GCC was bootstrapped successfully > targeting Thumb-1 and targeting Thumb-2 > > Is this ok for stage3? > > Best regards, > > Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline 2016-05-17 10:14 ` [PATCH, ARM 6/7, ping1] " Thomas Preudhomme @ 2016-05-20 13:23 ` Kyrill Tkachov 2016-07-06 16:55 ` Thomas Preudhomme 2016-07-12 10:27 ` Thomas Preudhomme 0 siblings, 2 replies; 7+ messages in thread From: Kyrill Tkachov @ 2016-05-20 13:23 UTC (permalink / raw) To: Thomas Preudhomme, Richard Earnshaw, Ramana Radhakrishnan; +Cc: gcc-patches Hi Thomas, On 17/05/16 11:14, Thomas Preudhomme wrote: > Ping? > > *** gcc/ChangeLog *** > > 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? valid > for Thumb-1. > * config/arm/arm.h (TARGET_HAVE_CBZ): Define. > (TARGET_IDIV): Set for all Thumb targets provided they have hardware > divide feature. > * config/arm/thumb1.md (thumb1_cbz): New insn. > > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > index > f42e996e5a7ce979fe406b8261d50fb2ba005f6b..347b5b0a5cc0bc1e3b5020c8124d968e76ce48a4 > 100644 > --- a/gcc/config/arm/arm.h > +++ b/gcc/config/arm/arm.h > @@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook) > (void); > /* Nonzero if this chip provides the movw and movt instructions. */ > #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) > > +/* Nonzero if this chip provides the cb{n}z instruction. */ > +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) > + > /* Nonzero if integer division instructions supported. */ > #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ > - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) > + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) > > /* Nonzero if disallow volatile memory access in IT block. */ > #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index > 13b4b71ac8f9c1da8ef1945f7ff6985ca59f6832..445972ce0e3fd27d4411840ff69e9edbb23994fc > 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -22684,7 +22684,7 @@ arm_print_operand_punct_valid_p (unsigned char code) > { > return (code == '@' || code == '|' || code == '.' > || code == '(' || code == ')' || code == '#' > - || (TARGET_32BIT && (code == '?')) > + || code == '?' > || (TARGET_THUMB2 && (code == '!')) > || (TARGET_THUMB && (code == '_'))); > } Hmm, I'm not a fan of this change. arm_print_operand_punct_valid_p is an implementation of a target hook that is used to validate user-provided inline asm as well and is therefore the right place to reject such invalid constructs. This is just working around the fact that the output template for the [u]divsi3 patterns has a '%?' in it that is illegal in Thumb1 and will not be used for ARMv8-M Baseline anyway. I'd prefer it if you add a second alternative to those patterns and emit the sdiv/udiv mnemonic without the '%?' and enable that for the v8mb arch attribute (and mark the existing alternative as requiring the "32" arch attribute). > diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md > index > 4572456b8bc98503061846cad94bc642943db3a2..1b01ef6ce731fe3ff37c3d8c048fb9d5e7829b35 > 100644 > --- a/gcc/config/arm/thumb1.md > +++ b/gcc/config/arm/thumb1.md > @@ -973,6 +973,92 @@ > DONE; > }) > > +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline profile, > +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads to > +;; code generation difference for ARMv6-M because the minimum length of the > +;; instruction becomes 2 even for it due to a limitation in genattrtab's > +;; handling of pc in the length condition. > +(define_insn "thumb1_cbz" > + [(set (pc) (if_then_else > + (match_operator 0 "equality_operator" > + [(match_operand:SI 1 "s_register_operand" "l") > + (const_int 0)]) > + (label_ref (match_operand 2 "" "")) > + (pc)))] > + "TARGET_THUMB1 && TARGET_HAVE_MOVT" > +{ s/TARGET_HAVE_MOVT/TARGET_HAVE_CBZ/ > + if (get_attr_length (insn) == 2) > + { > + if (GET_CODE (operands[0]) == EQ) > + return "cbz\t%1, %l2"; > + else > + return "cbnz\t%1, %l2"; > + } > + else > + { > + rtx t = cfun->machine->thumb1_cc_insn; > + if (t != NULL_RTX) > + { > + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) > + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) > + t = NULL_RTX; > + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) > + { > + if (!noov_comparison_operator (operands[0], VOIDmode)) > + t = NULL_RTX; > + } > + else if (cfun->machine->thumb1_cc_mode != CCmode) > + t = NULL_RTX; > + } > + if (t == NULL_RTX) > + { > + output_asm_insn ("cmp\t%1, #0", operands); > + cfun->machine->thumb1_cc_insn = insn; > + cfun->machine->thumb1_cc_op0 = operands[1]; > + cfun->machine->thumb1_cc_op1 = operands[2]; > + cfun->machine->thumb1_cc_mode = CCmode; > + } > + else > + /* Ensure we emit the right type of condition code on the jump. */ > + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, > + CC_REGNUM); > + > + switch (get_attr_length (insn)) > + { > + case 4: return "b%d0\t%l2"; > + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; > + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; > + default: gcc_unreachable (); > + } > + } > +} > + [(set (attr "far_jump") > + (if_then_else > + (eq_attr "length" "8") > + (const_string "yes") > + (const_string "no"))) > + (set (attr "length") > + (if_then_else > + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) > + (le (minus (match_dup 2) (pc)) (const_int 128)) > + (not (match_test "which_alternative"))) This pattern only has one alternative so "which_alternative" will always be 0, so the (not (match_test "which_alternative")) test inside the 'and' is redundant and can be removed. Thanks, Kyrill > + (const_int 2) > + (if_then_else > + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) > + (le (minus (match_dup 2) (pc)) (const_int 256))) > + (const_int 4) > + (if_then_else > + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) > + (le (minus (match_dup 2) (pc)) (const_int 2048))) > + (const_int 6) > + (const_int 8))))) > + (set (attr "type") > + (if_then_else > + (eq_attr "length" "2") > + (const_string "branch") > + (const_string "multiple")))] > +) > + > (define_insn "cbranchsi4_insn" > [(set (pc) (if_then_else > (match_operator 0 "arm_comparison_operator" > > > Best regards, > > Thomas > > On Thursday 17 December 2015 16:17:52 Thomas Preud'homme wrote: >> Hi, >> >> This patch is part of a patch series to add support for ARMv8-M[1] to GCC. >> This specific patch makes the compiler start generating code with the new >> CB(N)Z and (U|S)DIV instructions for ARMv8-M Baseline. >> >> Sharing of instruction patterns for div insn template with ARM or Thumb-2 >> was done by allowing %? punctuation character for Thumb-1. This is safe to >> do since the compiler would fault in arm_print_condition if a condition >> code is not handled by a branch in Thumb1. >> >> Unfortunately, cbz cannot be shared with cbranchsi4 because it would lead to >> worse code for Thumb-1. Indeed, choosing cb(n)z over the other alternatives >> for cbranchsi4 depends on the distance between target and pc which lead >> insn-attrtab to evaluate the minimum length of this pattern to be 2 as it >> cannot computer the distance statically. It would be possible to determine >> that this alternative is not available for non ARMv8-M Thumb-1 target >> statically but genattrtab is not currently capable to do it, so this is for >> a later patch. >> >> >> [1] For a quick overview of ARMv8-M please refer to the initial cover >> letter. >> >> ChangeLog entry is as follows: >> >> >> *** gcc/ChangeLog *** >> >> 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? valid >> for Thumb-1. >> * config/arm/arm.h (TARGET_HAVE_CBZ): Define. >> (TARGET_IDIV): Set for all Thumb targets provided they have hardware >> divide feature. >> * config/arm/thumb1.md (thumb1_cbz): New insn. >> >> >> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >> index 015df50..247f144 100644 >> --- a/gcc/config/arm/arm.h >> +++ b/gcc/config/arm/arm.h >> @@ -263,9 +263,12 @@ extern void >> (*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip >> provides the movw and movt instructions. */ #define >> TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) >> >> +/* Nonzero if this chip provides the cb{n}z instruction. */ >> +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) >> + >> /* Nonzero if integer division instructions supported. */ >> #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ >> - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) >> + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) >> >> /* Nonzero if disallow volatile memory access in IT block. */ >> #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >> index d832309..5ef3a1d 100644 >> --- a/gcc/config/arm/arm.c >> +++ b/gcc/config/arm/arm.c >> @@ -22568,7 +22568,7 @@ arm_print_operand_punct_valid_p (unsigned char code) >> { >> return (code == '@' || code == '|' || code == '.' >> >> || code == '(' || code == ')' || code == '#' >> >> - || (TARGET_32BIT && (code == '?')) >> + || code == '?' >> >> || (TARGET_THUMB2 && (code == '!')) >> || (TARGET_THUMB && (code == '_'))); >> >> } >> diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md >> index 7e3bcb4..074b267 100644 >> --- a/gcc/config/arm/thumb1.md >> +++ b/gcc/config/arm/thumb1.md >> @@ -973,6 +973,92 @@ >> DONE; >> }) >> >> +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline profile, >> +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads >> to +;; code generation difference for ARMv6-M because the minimum length of >> the +;; instruction becomes 2 even for it due to a limitation in >> genattrtab's +;; handling of pc in the length condition. >> +(define_insn "thumb1_cbz" >> + [(set (pc) (if_then_else >> + (match_operator 0 "equality_operator" >> + [(match_operand:SI 1 "s_register_operand" "l") >> + (const_int 0)]) >> + (label_ref (match_operand 2 "" "")) >> + (pc)))] >> + "TARGET_THUMB1 && TARGET_HAVE_MOVT" >> +{ >> + if (get_attr_length (insn) == 2) >> + { >> + if (GET_CODE (operands[0]) == EQ) >> + return "cbz\t%1, %l2"; >> + else >> + return "cbnz\t%1, %l2"; >> + } >> + else >> + { >> + rtx t = cfun->machine->thumb1_cc_insn; >> + if (t != NULL_RTX) >> + { >> + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) >> + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) >> + t = NULL_RTX; >> + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) >> + { >> + if (!noov_comparison_operator (operands[0], VOIDmode)) >> + t = NULL_RTX; >> + } >> + else if (cfun->machine->thumb1_cc_mode != CCmode) >> + t = NULL_RTX; >> + } >> + if (t == NULL_RTX) >> + { >> + output_asm_insn ("cmp\t%1, #0", operands); >> + cfun->machine->thumb1_cc_insn = insn; >> + cfun->machine->thumb1_cc_op0 = operands[1]; >> + cfun->machine->thumb1_cc_op1 = operands[2]; >> + cfun->machine->thumb1_cc_mode = CCmode; >> + } >> + else >> + /* Ensure we emit the right type of condition code on the jump. */ >> + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, >> + CC_REGNUM); >> + >> + switch (get_attr_length (insn)) >> + { >> + case 4: return "b%d0\t%l2"; >> + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; >> + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; >> + default: gcc_unreachable (); >> + } >> + } >> +} >> + [(set (attr "far_jump") >> + (if_then_else >> + (eq_attr "length" "8") >> + (const_string "yes") >> + (const_string "no"))) >> + (set (attr "length") >> + (if_then_else >> + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) >> + (le (minus (match_dup 2) (pc)) (const_int 128)) >> + (not (match_test "which_alternative"))) >> + (const_int 2) >> + (if_then_else >> + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) >> + (le (minus (match_dup 2) (pc)) (const_int 256))) >> + (const_int 4) >> + (if_then_else >> + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) >> + (le (minus (match_dup 2) (pc)) (const_int 2048))) >> + (const_int 6) >> + (const_int 8))))) >> + (set (attr "type") >> + (if_then_else >> + (eq_attr "length" "2") >> + (const_string "branch") >> + (const_string "multiple")))] >> +) >> + >> (define_insn "cbranchsi4_insn" >> [(set (pc) (if_then_else >> (match_operator 0 "arm_comparison_operator" >> >> >> Testing: >> >> * Toolchain was built successfully with and without the ARMv8-M support >> patches with the following multilib list: >> armv6-m,armv7-m,armv7e-m,cortex-m7. The code generation for crtbegin.o, >> crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a, >> libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a, libstdc++.a >> and libsupc++.a is unchanged for all these targets. >> >> * GCC also showed no testsuite regression when targeting ARMv8-M Baseline >> compared to ARMv6-M on ARM Fast Models and when targeting ARMv6-M and >> ARMv7-M (compared to without the patch) * GCC was bootstrapped successfully >> targeting Thumb-1 and targeting Thumb-2 >> >> Is this ok for stage3? >> >> Best regards, >> >> Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline 2016-05-20 13:23 ` Kyrill Tkachov @ 2016-07-06 16:55 ` Thomas Preudhomme 2016-07-12 10:27 ` Thomas Preudhomme 1 sibling, 0 replies; 7+ messages in thread From: Thomas Preudhomme @ 2016-07-06 16:55 UTC (permalink / raw) To: Kyrill Tkachov; +Cc: Richard Earnshaw, Ramana Radhakrishnan, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2278 bytes --] On Friday 20 May 2016 14:22:48 Kyrill Tkachov wrote: > Hi Thomas, > > > Hmm, I'm not a fan of this change. arm_print_operand_punct_valid_p is an > implementation of a target hook that is used to validate user-provided > inline asm as well and is therefore the right place to reject such invalid > constructs. > > This is just working around the fact that the output template for the > [u]divsi3 patterns has a '%?' in it that is illegal in Thumb1 and will not > be used for ARMv8-M Baseline anyway. I'd prefer it if you add a second > alternative to those patterns and emit the sdiv/udiv mnemonic without the > '%?' and enable that for the v8mb arch attribute (and mark the existing > alternative as requiring the "32" arch attribute). Fixed. > > s/TARGET_HAVE_MOVT/TARGET_HAVE_CBZ/ Likewise. > > + [(set (attr "far_jump") > > + (if_then_else > > + (eq_attr "length" "8") > > + (const_string "yes") > > + (const_string "no"))) > > + (set (attr "length") > > + (if_then_else > > + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) > > + (le (minus (match_dup 2) (pc)) (const_int 128)) > > + (not (match_test "which_alternative"))) > > This pattern only has one alternative so "which_alternative" > will always be 0, so the (not (match_test "which_alternative")) > test inside the 'and' is redundant and can be removed. Ditto. Please find updated ChangeLog entries: *** gcc/ChangeLog *** 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com> * config/arm/arm.h (TARGET_HAVE_CBZ): Define. (TARGET_IDIV): Set for all Thumb targets provided they have hardware divide feature. * config/arm/arm.md (divsi3): New unpredicable alternative for ARMv8-M Baseline. Make initial alternative TARGET_32BIT only. (udivsi3): Likewise. * config/arm/thumb1.md (thumb1_cbz): New insn. * doc/sourcebuild.texi (arm_thumb1_cbz_ok): Document new effective target. *** gcc/testsuite/ChangeLog *** 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com> * lib/target-supports.exp (check_effective_target_arm_thumb1_cbz_ok): Add new arm_thumb1_cbz_ok effective target. * gcc.target/arm/cbz.c: New test. Updated patch in attachment. Best regards, Thomas [-- Attachment #2: cbz_div_armv8m_support.diff --] [-- Type: text/x-patch, Size: 7531 bytes --] diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 5f8cfb07a841f7da39cd9e1c2c675dddb807a64f..4b8b7b6b96bdb697a3856ce4f56656b572c6bd22 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip provides the MOVW and MOVT instructions. */ #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) +/* Nonzero if this chip provides the CBZ and CBNZ instructions. */ +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) + /* Nonzero if integer division instructions supported. */ #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) /* Nonzero if disallow volatile memory access in IT block. */ #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 3f97c2ad68661e6a0f52ce4fb89f52ab73943a6e..b94c3626d4e82907eb9f81b9d56fc2ea006f9e08 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -4333,23 +4333,29 @@ ;; Division instructions (define_insn "divsi3" - [(set (match_operand:SI 0 "s_register_operand" "=r") - (div:SI (match_operand:SI 1 "s_register_operand" "r") - (match_operand:SI 2 "s_register_operand" "r")))] + [(set (match_operand:SI 0 "s_register_operand" "=r,r") + (div:SI (match_operand:SI 1 "s_register_operand" "r,r") + (match_operand:SI 2 "s_register_operand" "r,r")))] "TARGET_IDIV" - "sdiv%?\t%0, %1, %2" - [(set_attr "predicable" "yes") + "@ + sdiv%?\t%0, %1, %2 + sdiv\t%0, %1, %2" + [(set_attr "arch" "32,v8mb") + (set_attr "predicable" "yes") (set_attr "predicable_short_it" "no") (set_attr "type" "sdiv")] ) (define_insn "udivsi3" - [(set (match_operand:SI 0 "s_register_operand" "=r") - (udiv:SI (match_operand:SI 1 "s_register_operand" "r") - (match_operand:SI 2 "s_register_operand" "r")))] + [(set (match_operand:SI 0 "s_register_operand" "=r,r") + (udiv:SI (match_operand:SI 1 "s_register_operand" "r,r") + (match_operand:SI 2 "s_register_operand" "r,r")))] "TARGET_IDIV" - "udiv%?\t%0, %1, %2" - [(set_attr "predicable" "yes") + "@ + udiv%?\t%0, %1, %2 + udiv\t%0, %1, %2" + [(set_attr "arch" "32,v8mb") + (set_attr "predicable" "yes") (set_attr "predicable_short_it" "no") (set_attr "type" "udiv")] ) diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index 47e569d0c259cd17d86a03061e5b47b3dab4579f..0a8c364cc01aad34849f57879f9f18f9b92851c0 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -973,6 +973,91 @@ DONE; }) +;; A pattern for the CB(N)Z instruction added in ARMv8-M Baseline profile, +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads to +;; code generation difference for ARMv6-M because the minimum length of the +;; instruction becomes 2 even for ARMv6-M due to a limitation in genattrtab's +;; handling of PC in the length condition. +(define_insn "thumb1_cbz" + [(set (pc) (if_then_else + (match_operator 0 "equality_operator" + [(match_operand:SI 1 "s_register_operand" "l") + (const_int 0)]) + (label_ref (match_operand 2 "" "")) + (pc)))] + "TARGET_THUMB1 && TARGET_HAVE_CBZ" +{ + if (get_attr_length (insn) == 2) + { + if (GET_CODE (operands[0]) == EQ) + return "cbz\t%1, %l2"; + else + return "cbnz\t%1, %l2"; + } + else + { + rtx t = cfun->machine->thumb1_cc_insn; + if (t != NULL_RTX) + { + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) + t = NULL_RTX; + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) + { + if (!noov_comparison_operator (operands[0], VOIDmode)) + t = NULL_RTX; + } + else if (cfun->machine->thumb1_cc_mode != CCmode) + t = NULL_RTX; + } + if (t == NULL_RTX) + { + output_asm_insn ("cmp\t%1, #0", operands); + cfun->machine->thumb1_cc_insn = insn; + cfun->machine->thumb1_cc_op0 = operands[1]; + cfun->machine->thumb1_cc_op1 = operands[2]; + cfun->machine->thumb1_cc_mode = CCmode; + } + else + /* Ensure we emit the right type of condition code on the jump. */ + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, + CC_REGNUM); + + switch (get_attr_length (insn)) + { + case 4: return "b%d0\t%l2"; + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; + default: gcc_unreachable (); + } + } +} + [(set (attr "far_jump") + (if_then_else + (eq_attr "length" "8") + (const_string "yes") + (const_string "no"))) + (set (attr "length") + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) + (le (minus (match_dup 2) (pc)) (const_int 128))) + (const_int 2) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) + (le (minus (match_dup 2) (pc)) (const_int 256))) + (const_int 4) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) + (le (minus (match_dup 2) (pc)) (const_int 2048))) + (const_int 6) + (const_int 8))))) + (set (attr "type") + (if_then_else + (eq_attr "length" "2") + (const_string "branch") + (const_string "multiple")))] +) + (define_insn "cbranchsi4_insn" [(set (pc) (if_then_else (match_operator 0 "arm_comparison_operator" diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index 1fcd464c27c87ec89d1621a94fe72aa8ae5a5a0a..72a7c020d6d25ffe152d9618281c2be632b57a27 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -1605,6 +1605,10 @@ ARM target prefers @code{LDRD} and @code{STRD} instructions over ARM target generates Thumb-1 code for @code{-mthumb} with @code{MOVW} and @code{MOVT} instructions available. +@item arm_thumb1_cbz_ok +ARM target generates Thumb-1 code for @code{-mthumb} with +@code{CBZ} and @code{CBNZ} instructions available. + @end table @subsubsection AArch64-specific attributes diff --git a/gcc/testsuite/gcc.target/arm/cbz.c b/gcc/testsuite/gcc.target/arm/cbz.c new file mode 100644 index 0000000000000000000000000000000000000000..5d3de6387777743883a3baf66b878a8a0c3f1783 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cbz.c @@ -0,0 +1,12 @@ +/* { dg-do compile {target { arm_thumb2 || arm_thumb1_cbz_ok } } } */ +/* { dg-options "-O2" } */ + +int +foo (int a, int *b) +{ + if (a) + *b = 1; + return 0; +} + +/* { dg-final { scan-assembler-times "cbz\\tr\\d" 1 } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index b71007e5ebcd925c5c27d58647deee8c53c43600..127af0a1f33bebb0ecc6aa8587d992e89524ce6e 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -3346,6 +3346,23 @@ proc check_effective_target_arm_thumb1_movt_ok {} { } } +# Return 1 if this is an ARM target where -mthumb causes Thumb-1 to be +# used and CBZ and CBNZ instructions are available. + +proc check_effective_target_arm_thumb1_cbz_ok {} { + if [check_effective_target_arm_thumb1_ok] { + return [check_no_compiler_messages arm_movt object { + int + foo (void) + { + asm ("cbz r0, 2f\n2:"); + } + } "-mthumb"] + } else { + return 0 + } +} + # Return 1 if this compilation turns on string_ops_prefer_neon on. proc check_effective_target_arm_tune_string_ops_prefer_neon { } { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline 2016-05-20 13:23 ` Kyrill Tkachov 2016-07-06 16:55 ` Thomas Preudhomme @ 2016-07-12 10:27 ` Thomas Preudhomme 2016-07-12 14:57 ` Kyrill Tkachov 1 sibling, 1 reply; 7+ messages in thread From: Thomas Preudhomme @ 2016-07-12 10:27 UTC (permalink / raw) To: Kyrill Tkachov; +Cc: Richard Earnshaw, Ramana Radhakrishnan, gcc-patches [-- Attachment #1: Type: text/plain, Size: 15412 bytes --] Hi Kyrill, On Friday 20 May 2016 14:22:48 Kyrill Tkachov wrote: > Hi Thomas, > > On 17/05/16 11:14, Thomas Preudhomme wrote: > > Ping? > > > > *** gcc/ChangeLog *** > > > > 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> > > > > * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? > > valid > > for Thumb-1. > > * config/arm/arm.h (TARGET_HAVE_CBZ): Define. > > (TARGET_IDIV): Set for all Thumb targets provided they have > > hardware > > divide feature. > > * config/arm/thumb1.md (thumb1_cbz): New insn. > > > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > > index > > f42e996e5a7ce979fe406b8261d50fb2ba005f6b..347b5b0a5cc0bc1e3b5020c8124d968e > > 76ce48a4 100644 > > --- a/gcc/config/arm/arm.h > > +++ b/gcc/config/arm/arm.h > > @@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook) > > (void); > > > > /* Nonzero if this chip provides the movw and movt instructions. */ > > #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) > > > > +/* Nonzero if this chip provides the cb{n}z instruction. */ > > +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) > > + > > > > /* Nonzero if integer division instructions supported. */ > > #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ > > > > - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) > > + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) > > > > /* Nonzero if disallow volatile memory access in IT block. */ > > #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) > > > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > > index > > 13b4b71ac8f9c1da8ef1945f7ff6985ca59f6832..445972ce0e3fd27d4411840ff69e9edb > > b23994fc 100644 > > --- a/gcc/config/arm/arm.c > > +++ b/gcc/config/arm/arm.c > > @@ -22684,7 +22684,7 @@ arm_print_operand_punct_valid_p (unsigned char > > code)> > > { > > > > return (code == '@' || code == '|' || code == '.' > > > > || code == '(' || code == ')' || code == '#' > > > > - || (TARGET_32BIT && (code == '?')) > > + || code == '?' > > > > || (TARGET_THUMB2 && (code == '!')) > > || (TARGET_THUMB && (code == '_'))); > > > > } > > Hmm, I'm not a fan of this change. arm_print_operand_punct_valid_p is an > implementation of a target hook that is used to validate user-provided > inline asm as well and is therefore the right place to reject such invalid > constructs. > > This is just working around the fact that the output template for the > [u]divsi3 patterns has a '%?' in it that is illegal in Thumb1 and will not > be used for ARMv8-M Baseline anyway. I'd prefer it if you add a second > alternative to those patterns and emit the sdiv/udiv mnemonic without the > '%?' and enable that for the v8mb arch attribute (and mark the existing > alternative as requiring the "32" arch attribute). > > diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md > > index > > 4572456b8bc98503061846cad94bc642943db3a2..1b01ef6ce731fe3ff37c3d8c048fb9d5 > > e7829b35 100644 > > --- a/gcc/config/arm/thumb1.md > > +++ b/gcc/config/arm/thumb1.md > > @@ -973,6 +973,92 @@ > > > > DONE; > > > > }) > > > > +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline > > profile, > > +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads > > to +;; code generation difference for ARMv6-M because the minimum length > > of the +;; instruction becomes 2 even for it due to a limitation in > > genattrtab's +;; handling of pc in the length condition. > > +(define_insn "thumb1_cbz" > > + [(set (pc) (if_then_else > > + (match_operator 0 "equality_operator" > > + [(match_operand:SI 1 "s_register_operand" "l") > > + (const_int 0)]) > > + (label_ref (match_operand 2 "" "")) > > + (pc)))] > > + "TARGET_THUMB1 && TARGET_HAVE_MOVT" > > +{ > > s/TARGET_HAVE_MOVT/TARGET_HAVE_CBZ/ > > > + if (get_attr_length (insn) == 2) > > + { > > + if (GET_CODE (operands[0]) == EQ) > > + return "cbz\t%1, %l2"; > > + else > > + return "cbnz\t%1, %l2"; > > + } > > + else > > + { > > + rtx t = cfun->machine->thumb1_cc_insn; > > + if (t != NULL_RTX) > > + { > > + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) > > + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) > > + t = NULL_RTX; > > + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) > > + { > > + if (!noov_comparison_operator (operands[0], VOIDmode)) > > + t = NULL_RTX; > > + } > > + else if (cfun->machine->thumb1_cc_mode != CCmode) > > + t = NULL_RTX; > > + } > > + if (t == NULL_RTX) > > + { > > + output_asm_insn ("cmp\t%1, #0", operands); > > + cfun->machine->thumb1_cc_insn = insn; > > + cfun->machine->thumb1_cc_op0 = operands[1]; > > + cfun->machine->thumb1_cc_op1 = operands[2]; > > + cfun->machine->thumb1_cc_mode = CCmode; > > + } > > + else > > + /* Ensure we emit the right type of condition code on the jump. */ > > + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, > > + CC_REGNUM); > > + > > + switch (get_attr_length (insn)) > > + { > > + case 4: return "b%d0\t%l2"; > > + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; > > + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; > > + default: gcc_unreachable (); > > + } > > + } > > +} > > + [(set (attr "far_jump") > > + (if_then_else > > + (eq_attr "length" "8") > > + (const_string "yes") > > + (const_string "no"))) > > + (set (attr "length") > > + (if_then_else > > + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) > > + (le (minus (match_dup 2) (pc)) (const_int 128)) > > + (not (match_test "which_alternative"))) > > This pattern only has one alternative so "which_alternative" > will always be 0, so the (not (match_test "which_alternative")) > test inside the 'and' is redundant and can be removed. What about the attached updated patch? ChangeLog entry are as follow: *** gcc/ChangeLog *** 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com> * config/arm/arm.h (TARGET_HAVE_CBZ): Define. (TARGET_IDIV): Set for all Thumb targets provided they have hardware divide feature. * config/arm/arm.md (divsi3): New unpredicable alternative for ARMv8-M Baseline. Make initial alternative TARGET_32BIT only. (udivsi3): Likewise. * config/arm/thumb1.md (thumb1_cbz): New insn. * doc/sourcebuild.texi (arm_thumb1_cbz_ok): Document new effective target. *** gcc/testsuite/ChangeLog *** 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com> * lib/target-supports.exp (check_effective_target_arm_thumb1_cbz_ok): Add new arm_thumb1_cbz_ok effective target. * gcc.target/arm/cbz.c: New test. Best regards, Thomas > > Thanks, > Kyrill > > > + (const_int 2) > > + (if_then_else > > + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) > > + (le (minus (match_dup 2) (pc)) (const_int 256))) > > + (const_int 4) > > + (if_then_else > > + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) > > + (le (minus (match_dup 2) (pc)) (const_int 2048))) > > + (const_int 6) > > + (const_int 8))))) > > + (set (attr "type") > > + (if_then_else > > + (eq_attr "length" "2") > > + (const_string "branch") > > + (const_string "multiple")))] > > +) > > + > > > > (define_insn "cbranchsi4_insn" > > > > [(set (pc) (if_then_else > > > > (match_operator 0 "arm_comparison_operator" > > > > Best regards, > > > > Thomas > > > > On Thursday 17 December 2015 16:17:52 Thomas Preud'homme wrote: > >> Hi, > >> > >> This patch is part of a patch series to add support for ARMv8-M[1] to > >> GCC. > >> This specific patch makes the compiler start generating code with the new > >> CB(N)Z and (U|S)DIV instructions for ARMv8-M Baseline. > >> > >> Sharing of instruction patterns for div insn template with ARM or Thumb-2 > >> was done by allowing %? punctuation character for Thumb-1. This is safe > >> to > >> do since the compiler would fault in arm_print_condition if a condition > >> code is not handled by a branch in Thumb1. > >> > >> Unfortunately, cbz cannot be shared with cbranchsi4 because it would lead > >> to worse code for Thumb-1. Indeed, choosing cb(n)z over the other > >> alternatives for cbranchsi4 depends on the distance between target and > >> pc which lead insn-attrtab to evaluate the minimum length of this > >> pattern to be 2 as it cannot computer the distance statically. It would > >> be possible to determine that this alternative is not available for non > >> ARMv8-M Thumb-1 target statically but genattrtab is not currently > >> capable to do it, so this is for a later patch. > >> > >> > >> [1] For a quick overview of ARMv8-M please refer to the initial cover > >> letter. > >> > >> ChangeLog entry is as follows: > >> > >> > >> *** gcc/ChangeLog *** > >> > >> 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> > >> > >> * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? > >> valid > >> for Thumb-1. > >> * config/arm/arm.h (TARGET_HAVE_CBZ): Define. > >> (TARGET_IDIV): Set for all Thumb targets provided they have > >> hardware > >> > >> divide feature. > >> > >> * config/arm/thumb1.md (thumb1_cbz): New insn. > >> > >> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > >> index 015df50..247f144 100644 > >> --- a/gcc/config/arm/arm.h > >> +++ b/gcc/config/arm/arm.h > >> @@ -263,9 +263,12 @@ extern void > >> (*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip > >> provides the movw and movt instructions. */ #define > >> TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) > >> > >> +/* Nonzero if this chip provides the cb{n}z instruction. */ > >> +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) > >> + > >> > >> /* Nonzero if integer division instructions supported. */ > >> #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ > >> > >> - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) > >> + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) > >> > >> /* Nonzero if disallow volatile memory access in IT block. */ > >> #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) > >> > >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > >> index d832309..5ef3a1d 100644 > >> --- a/gcc/config/arm/arm.c > >> +++ b/gcc/config/arm/arm.c > >> @@ -22568,7 +22568,7 @@ arm_print_operand_punct_valid_p (unsigned char > >> code) { > >> > >> return (code == '@' || code == '|' || code == '.' > >> > >> || code == '(' || code == ')' || code == '#' > >> > >> - || (TARGET_32BIT && (code == '?')) > >> + || code == '?' > >> > >> || (TARGET_THUMB2 && (code == '!')) > >> || (TARGET_THUMB && (code == '_'))); > >> > >> } > >> > >> diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md > >> index 7e3bcb4..074b267 100644 > >> --- a/gcc/config/arm/thumb1.md > >> +++ b/gcc/config/arm/thumb1.md > >> @@ -973,6 +973,92 @@ > >> > >> DONE; > >> > >> }) > >> > >> +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline > >> profile, > >> +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead > >> leads > >> to +;; code generation difference for ARMv6-M because the minimum length > >> of > >> the +;; instruction becomes 2 even for it due to a limitation in > >> genattrtab's +;; handling of pc in the length condition. > >> +(define_insn "thumb1_cbz" > >> + [(set (pc) (if_then_else > >> + (match_operator 0 "equality_operator" > >> + [(match_operand:SI 1 "s_register_operand" "l") > >> + (const_int 0)]) > >> + (label_ref (match_operand 2 "" "")) > >> + (pc)))] > >> + "TARGET_THUMB1 && TARGET_HAVE_MOVT" > >> +{ > >> + if (get_attr_length (insn) == 2) > >> + { > >> + if (GET_CODE (operands[0]) == EQ) > >> + return "cbz\t%1, %l2"; > >> + else > >> + return "cbnz\t%1, %l2"; > >> + } > >> + else > >> + { > >> + rtx t = cfun->machine->thumb1_cc_insn; > >> + if (t != NULL_RTX) > >> + { > >> + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) > >> + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) > >> + t = NULL_RTX; > >> + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) > >> + { > >> + if (!noov_comparison_operator (operands[0], VOIDmode)) > >> + t = NULL_RTX; > >> + } > >> + else if (cfun->machine->thumb1_cc_mode != CCmode) > >> + t = NULL_RTX; > >> + } > >> + if (t == NULL_RTX) > >> + { > >> + output_asm_insn ("cmp\t%1, #0", operands); > >> + cfun->machine->thumb1_cc_insn = insn; > >> + cfun->machine->thumb1_cc_op0 = operands[1]; > >> + cfun->machine->thumb1_cc_op1 = operands[2]; > >> + cfun->machine->thumb1_cc_mode = CCmode; > >> + } > >> + else > >> + /* Ensure we emit the right type of condition code on the jump. */ > >> + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, > >> + CC_REGNUM); > >> + > >> + switch (get_attr_length (insn)) > >> + { > >> + case 4: return "b%d0\t%l2"; > >> + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; > >> + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; > >> + default: gcc_unreachable (); > >> + } > >> + } > >> +} > >> + [(set (attr "far_jump") > >> + (if_then_else > >> + (eq_attr "length" "8") > >> + (const_string "yes") > >> + (const_string "no"))) > >> + (set (attr "length") > >> + (if_then_else > >> + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) > >> + (le (minus (match_dup 2) (pc)) (const_int 128)) > >> + (not (match_test "which_alternative"))) > >> + (const_int 2) > >> + (if_then_else > >> + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) > >> + (le (minus (match_dup 2) (pc)) (const_int 256))) > >> + (const_int 4) > >> + (if_then_else > >> + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) > >> + (le (minus (match_dup 2) (pc)) (const_int 2048))) > >> + (const_int 6) > >> + (const_int 8))))) > >> + (set (attr "type") > >> + (if_then_else > >> + (eq_attr "length" "2") > >> + (const_string "branch") > >> + (const_string "multiple")))] > >> +) > >> + > >> > >> (define_insn "cbranchsi4_insn" > >> > >> [(set (pc) (if_then_else > >> > >> (match_operator 0 "arm_comparison_operator" > >> > >> Testing: > >> > >> * Toolchain was built successfully with and without the ARMv8-M support > >> patches with the following multilib list: > >> armv6-m,armv7-m,armv7e-m,cortex-m7. The code generation for crtbegin.o, > >> crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a, > >> libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a, > >> libstdc++.a > >> and libsupc++.a is unchanged for all these targets. > >> > >> * GCC also showed no testsuite regression when targeting ARMv8-M Baseline > >> compared to ARMv6-M on ARM Fast Models and when targeting ARMv6-M and > >> ARMv7-M (compared to without the patch) * GCC was bootstrapped > >> successfully > >> targeting Thumb-1 and targeting Thumb-2 > >> > >> Is this ok for stage3? > >> > >> Best regards, > >> > >> Thomas [-- Attachment #2: enable_cbz_div_armv8m_baseline.patch --] [-- Type: text/x-patch, Size: 7531 bytes --] diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 5f8cfb07a841f7da39cd9e1c2c675dddb807a64f..4b8b7b6b96bdb697a3856ce4f56656b572c6bd22 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip provides the MOVW and MOVT instructions. */ #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) +/* Nonzero if this chip provides the CBZ and CBNZ instructions. */ +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) + /* Nonzero if integer division instructions supported. */ #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) /* Nonzero if disallow volatile memory access in IT block. */ #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 3f97c2ad68661e6a0f52ce4fb89f52ab73943a6e..b94c3626d4e82907eb9f81b9d56fc2ea006f9e08 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -4333,23 +4333,29 @@ ;; Division instructions (define_insn "divsi3" - [(set (match_operand:SI 0 "s_register_operand" "=r") - (div:SI (match_operand:SI 1 "s_register_operand" "r") - (match_operand:SI 2 "s_register_operand" "r")))] + [(set (match_operand:SI 0 "s_register_operand" "=r,r") + (div:SI (match_operand:SI 1 "s_register_operand" "r,r") + (match_operand:SI 2 "s_register_operand" "r,r")))] "TARGET_IDIV" - "sdiv%?\t%0, %1, %2" - [(set_attr "predicable" "yes") + "@ + sdiv%?\t%0, %1, %2 + sdiv\t%0, %1, %2" + [(set_attr "arch" "32,v8mb") + (set_attr "predicable" "yes") (set_attr "predicable_short_it" "no") (set_attr "type" "sdiv")] ) (define_insn "udivsi3" - [(set (match_operand:SI 0 "s_register_operand" "=r") - (udiv:SI (match_operand:SI 1 "s_register_operand" "r") - (match_operand:SI 2 "s_register_operand" "r")))] + [(set (match_operand:SI 0 "s_register_operand" "=r,r") + (udiv:SI (match_operand:SI 1 "s_register_operand" "r,r") + (match_operand:SI 2 "s_register_operand" "r,r")))] "TARGET_IDIV" - "udiv%?\t%0, %1, %2" - [(set_attr "predicable" "yes") + "@ + udiv%?\t%0, %1, %2 + udiv\t%0, %1, %2" + [(set_attr "arch" "32,v8mb") + (set_attr "predicable" "yes") (set_attr "predicable_short_it" "no") (set_attr "type" "udiv")] ) diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md index 47e569d0c259cd17d86a03061e5b47b3dab4579f..0a8c364cc01aad34849f57879f9f18f9b92851c0 100644 --- a/gcc/config/arm/thumb1.md +++ b/gcc/config/arm/thumb1.md @@ -973,6 +973,91 @@ DONE; }) +;; A pattern for the CB(N)Z instruction added in ARMv8-M Baseline profile, +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads to +;; code generation difference for ARMv6-M because the minimum length of the +;; instruction becomes 2 even for ARMv6-M due to a limitation in genattrtab's +;; handling of PC in the length condition. +(define_insn "thumb1_cbz" + [(set (pc) (if_then_else + (match_operator 0 "equality_operator" + [(match_operand:SI 1 "s_register_operand" "l") + (const_int 0)]) + (label_ref (match_operand 2 "" "")) + (pc)))] + "TARGET_THUMB1 && TARGET_HAVE_CBZ" +{ + if (get_attr_length (insn) == 2) + { + if (GET_CODE (operands[0]) == EQ) + return "cbz\t%1, %l2"; + else + return "cbnz\t%1, %l2"; + } + else + { + rtx t = cfun->machine->thumb1_cc_insn; + if (t != NULL_RTX) + { + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) + t = NULL_RTX; + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) + { + if (!noov_comparison_operator (operands[0], VOIDmode)) + t = NULL_RTX; + } + else if (cfun->machine->thumb1_cc_mode != CCmode) + t = NULL_RTX; + } + if (t == NULL_RTX) + { + output_asm_insn ("cmp\t%1, #0", operands); + cfun->machine->thumb1_cc_insn = insn; + cfun->machine->thumb1_cc_op0 = operands[1]; + cfun->machine->thumb1_cc_op1 = operands[2]; + cfun->machine->thumb1_cc_mode = CCmode; + } + else + /* Ensure we emit the right type of condition code on the jump. */ + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, + CC_REGNUM); + + switch (get_attr_length (insn)) + { + case 4: return "b%d0\t%l2"; + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; + default: gcc_unreachable (); + } + } +} + [(set (attr "far_jump") + (if_then_else + (eq_attr "length" "8") + (const_string "yes") + (const_string "no"))) + (set (attr "length") + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) + (le (minus (match_dup 2) (pc)) (const_int 128))) + (const_int 2) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) + (le (minus (match_dup 2) (pc)) (const_int 256))) + (const_int 4) + (if_then_else + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) + (le (minus (match_dup 2) (pc)) (const_int 2048))) + (const_int 6) + (const_int 8))))) + (set (attr "type") + (if_then_else + (eq_attr "length" "2") + (const_string "branch") + (const_string "multiple")))] +) + (define_insn "cbranchsi4_insn" [(set (pc) (if_then_else (match_operator 0 "arm_comparison_operator" diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index 1fcd464c27c87ec89d1621a94fe72aa8ae5a5a0a..72a7c020d6d25ffe152d9618281c2be632b57a27 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -1605,6 +1605,10 @@ ARM target prefers @code{LDRD} and @code{STRD} instructions over ARM target generates Thumb-1 code for @code{-mthumb} with @code{MOVW} and @code{MOVT} instructions available. +@item arm_thumb1_cbz_ok +ARM target generates Thumb-1 code for @code{-mthumb} with +@code{CBZ} and @code{CBNZ} instructions available. + @end table @subsubsection AArch64-specific attributes diff --git a/gcc/testsuite/gcc.target/arm/cbz.c b/gcc/testsuite/gcc.target/arm/cbz.c new file mode 100644 index 0000000000000000000000000000000000000000..5d3de6387777743883a3baf66b878a8a0c3f1783 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/cbz.c @@ -0,0 +1,12 @@ +/* { dg-do compile {target { arm_thumb2 || arm_thumb1_cbz_ok } } } */ +/* { dg-options "-O2" } */ + +int +foo (int a, int *b) +{ + if (a) + *b = 1; + return 0; +} + +/* { dg-final { scan-assembler-times "cbz\\tr\\d" 1 } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index b71007e5ebcd925c5c27d58647deee8c53c43600..127af0a1f33bebb0ecc6aa8587d992e89524ce6e 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -3346,6 +3346,23 @@ proc check_effective_target_arm_thumb1_movt_ok {} { } } +# Return 1 if this is an ARM target where -mthumb causes Thumb-1 to be +# used and CBZ and CBNZ instructions are available. + +proc check_effective_target_arm_thumb1_cbz_ok {} { + if [check_effective_target_arm_thumb1_ok] { + return [check_no_compiler_messages arm_movt object { + int + foo (void) + { + asm ("cbz r0, 2f\n2:"); + } + } "-mthumb"] + } else { + return 0 + } +} + # Return 1 if this compilation turns on string_ops_prefer_neon on. proc check_effective_target_arm_tune_string_ops_prefer_neon { } { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline 2016-07-12 10:27 ` Thomas Preudhomme @ 2016-07-12 14:57 ` Kyrill Tkachov 2016-07-13 10:54 ` Thomas Preudhomme 0 siblings, 1 reply; 7+ messages in thread From: Kyrill Tkachov @ 2016-07-12 14:57 UTC (permalink / raw) To: Thomas Preudhomme; +Cc: Richard Earnshaw, Ramana Radhakrishnan, gcc-patches On 12/07/16 11:26, Thomas Preudhomme wrote: > Hi Kyrill, Hi Thomas, > > On Friday 20 May 2016 14:22:48 Kyrill Tkachov wrote: >> Hi Thomas, >> >> On 17/05/16 11:14, Thomas Preudhomme wrote: >>> Ping? >>> >>> *** gcc/ChangeLog *** >>> >>> 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> >>> >>> * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? >>> valid >>> for Thumb-1. >>> * config/arm/arm.h (TARGET_HAVE_CBZ): Define. >>> (TARGET_IDIV): Set for all Thumb targets provided they have >>> hardware >>> divide feature. >>> * config/arm/thumb1.md (thumb1_cbz): New insn. >>> >>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >>> index >>> f42e996e5a7ce979fe406b8261d50fb2ba005f6b..347b5b0a5cc0bc1e3b5020c8124d968e >>> 76ce48a4 100644 >>> --- a/gcc/config/arm/arm.h >>> +++ b/gcc/config/arm/arm.h >>> @@ -271,9 +271,12 @@ extern void (*arm_lang_output_object_attributes_hook) >>> (void); >>> >>> /* Nonzero if this chip provides the movw and movt instructions. */ >>> #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) >>> >>> +/* Nonzero if this chip provides the cb{n}z instruction. */ >>> +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) >>> + >>> >>> /* Nonzero if integer division instructions supported. */ >>> #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ >>> >>> - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) >>> + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) >>> >>> /* Nonzero if disallow volatile memory access in IT block. */ >>> #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) >>> >>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >>> index >>> 13b4b71ac8f9c1da8ef1945f7ff6985ca59f6832..445972ce0e3fd27d4411840ff69e9edb >>> b23994fc 100644 >>> --- a/gcc/config/arm/arm.c >>> +++ b/gcc/config/arm/arm.c >>> @@ -22684,7 +22684,7 @@ arm_print_operand_punct_valid_p (unsigned char >>> code)> >>> { >>> >>> return (code == '@' || code == '|' || code == '.' >>> >>> || code == '(' || code == ')' || code == '#' >>> >>> - || (TARGET_32BIT && (code == '?')) >>> + || code == '?' >>> >>> || (TARGET_THUMB2 && (code == '!')) >>> || (TARGET_THUMB && (code == '_'))); >>> >>> } >> Hmm, I'm not a fan of this change. arm_print_operand_punct_valid_p is an >> implementation of a target hook that is used to validate user-provided >> inline asm as well and is therefore the right place to reject such invalid >> constructs. >> >> This is just working around the fact that the output template for the >> [u]divsi3 patterns has a '%?' in it that is illegal in Thumb1 and will not >> be used for ARMv8-M Baseline anyway. I'd prefer it if you add a second >> alternative to those patterns and emit the sdiv/udiv mnemonic without the >> '%?' and enable that for the v8mb arch attribute (and mark the existing >> alternative as requiring the "32" arch attribute). >>> diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md >>> index >>> 4572456b8bc98503061846cad94bc642943db3a2..1b01ef6ce731fe3ff37c3d8c048fb9d5 >>> e7829b35 100644 >>> --- a/gcc/config/arm/thumb1.md >>> +++ b/gcc/config/arm/thumb1.md >>> @@ -973,6 +973,92 @@ >>> >>> DONE; >>> >>> }) >>> >>> +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline >>> profile, >>> +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead leads >>> to +;; code generation difference for ARMv6-M because the minimum length >>> of the +;; instruction becomes 2 even for it due to a limitation in >>> genattrtab's +;; handling of pc in the length condition. >>> +(define_insn "thumb1_cbz" >>> + [(set (pc) (if_then_else >>> + (match_operator 0 "equality_operator" >>> + [(match_operand:SI 1 "s_register_operand" "l") >>> + (const_int 0)]) >>> + (label_ref (match_operand 2 "" "")) >>> + (pc)))] >>> + "TARGET_THUMB1 && TARGET_HAVE_MOVT" >>> +{ >> s/TARGET_HAVE_MOVT/TARGET_HAVE_CBZ/ >> >>> + if (get_attr_length (insn) == 2) >>> + { >>> + if (GET_CODE (operands[0]) == EQ) >>> + return "cbz\t%1, %l2"; >>> + else >>> + return "cbnz\t%1, %l2"; >>> + } >>> + else >>> + { >>> + rtx t = cfun->machine->thumb1_cc_insn; >>> + if (t != NULL_RTX) >>> + { >>> + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) >>> + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) >>> + t = NULL_RTX; >>> + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) >>> + { >>> + if (!noov_comparison_operator (operands[0], VOIDmode)) >>> + t = NULL_RTX; >>> + } >>> + else if (cfun->machine->thumb1_cc_mode != CCmode) >>> + t = NULL_RTX; >>> + } >>> + if (t == NULL_RTX) >>> + { >>> + output_asm_insn ("cmp\t%1, #0", operands); >>> + cfun->machine->thumb1_cc_insn = insn; >>> + cfun->machine->thumb1_cc_op0 = operands[1]; >>> + cfun->machine->thumb1_cc_op1 = operands[2]; >>> + cfun->machine->thumb1_cc_mode = CCmode; >>> + } >>> + else >>> + /* Ensure we emit the right type of condition code on the jump. */ >>> + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, >>> + CC_REGNUM); >>> + >>> + switch (get_attr_length (insn)) >>> + { >>> + case 4: return "b%d0\t%l2"; >>> + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; >>> + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; >>> + default: gcc_unreachable (); >>> + } >>> + } >>> +} >>> + [(set (attr "far_jump") >>> + (if_then_else >>> + (eq_attr "length" "8") >>> + (const_string "yes") >>> + (const_string "no"))) >>> + (set (attr "length") >>> + (if_then_else >>> + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) >>> + (le (minus (match_dup 2) (pc)) (const_int 128)) >>> + (not (match_test "which_alternative"))) >> This pattern only has one alternative so "which_alternative" >> will always be 0, so the (not (match_test "which_alternative")) >> test inside the 'and' is redundant and can be removed. > What about the attached updated patch? This is ok with one ChangeLog nit. Thanks, Kyrill > ChangeLog entry are as follow: > > > *** gcc/ChangeLog *** > > 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/arm.h (TARGET_HAVE_CBZ): Define. > (TARGET_IDIV): Set for all Thumb targets provided they have hardware > divide feature. > * config/arm/arm.md (divsi3): New unpredicable alternative for ARMv8-M > Baseline. Make initial alternative TARGET_32BIT only. > (udivsi3): Likewise. > * config/arm/thumb1.md (thumb1_cbz): New insn. I'd say "New define_insn." instead. > * doc/sourcebuild.texi (arm_thumb1_cbz_ok): Document new effective > target. > > > *** gcc/testsuite/ChangeLog *** > > 2016-05-23 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * lib/target-supports.exp (check_effective_target_arm_thumb1_cbz_ok): > Add new arm_thumb1_cbz_ok effective target. > * gcc.target/arm/cbz.c: New test. > > > Best regards, > > Thomas > >> Thanks, >> Kyrill >> >>> + (const_int 2) >>> + (if_then_else >>> + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) >>> + (le (minus (match_dup 2) (pc)) (const_int 256))) >>> + (const_int 4) >>> + (if_then_else >>> + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) >>> + (le (minus (match_dup 2) (pc)) (const_int 2048))) >>> + (const_int 6) >>> + (const_int 8))))) >>> + (set (attr "type") >>> + (if_then_else >>> + (eq_attr "length" "2") >>> + (const_string "branch") >>> + (const_string "multiple")))] >>> +) >>> + >>> >>> (define_insn "cbranchsi4_insn" >>> >>> [(set (pc) (if_then_else >>> >>> (match_operator 0 "arm_comparison_operator" >>> >>> Best regards, >>> >>> Thomas >>> >>> On Thursday 17 December 2015 16:17:52 Thomas Preud'homme wrote: >>>> Hi, >>>> >>>> This patch is part of a patch series to add support for ARMv8-M[1] to >>>> GCC. >>>> This specific patch makes the compiler start generating code with the new >>>> CB(N)Z and (U|S)DIV instructions for ARMv8-M Baseline. >>>> >>>> Sharing of instruction patterns for div insn template with ARM or Thumb-2 >>>> was done by allowing %? punctuation character for Thumb-1. This is safe >>>> to >>>> do since the compiler would fault in arm_print_condition if a condition >>>> code is not handled by a branch in Thumb1. >>>> >>>> Unfortunately, cbz cannot be shared with cbranchsi4 because it would lead >>>> to worse code for Thumb-1. Indeed, choosing cb(n)z over the other >>>> alternatives for cbranchsi4 depends on the distance between target and >>>> pc which lead insn-attrtab to evaluate the minimum length of this >>>> pattern to be 2 as it cannot computer the distance statically. It would >>>> be possible to determine that this alternative is not available for non >>>> ARMv8-M Thumb-1 target statically but genattrtab is not currently >>>> capable to do it, so this is for a later patch. >>>> >>>> >>>> [1] For a quick overview of ARMv8-M please refer to the initial cover >>>> letter. >>>> >>>> ChangeLog entry is as follows: >>>> >>>> >>>> *** gcc/ChangeLog *** >>>> >>>> 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> >>>> >>>> * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? >>>> valid >>>> for Thumb-1. >>>> * config/arm/arm.h (TARGET_HAVE_CBZ): Define. >>>> (TARGET_IDIV): Set for all Thumb targets provided they have >>>> hardware >>>> >>>> divide feature. >>>> >>>> * config/arm/thumb1.md (thumb1_cbz): New insn. >>>> >>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >>>> index 015df50..247f144 100644 >>>> --- a/gcc/config/arm/arm.h >>>> +++ b/gcc/config/arm/arm.h >>>> @@ -263,9 +263,12 @@ extern void >>>> (*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip >>>> provides the movw and movt instructions. */ #define >>>> TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) >>>> >>>> +/* Nonzero if this chip provides the cb{n}z instruction. */ >>>> +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) >>>> + >>>> >>>> /* Nonzero if integer division instructions supported. */ >>>> #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ >>>> >>>> - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) >>>> + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) >>>> >>>> /* Nonzero if disallow volatile memory access in IT block. */ >>>> #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) >>>> >>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >>>> index d832309..5ef3a1d 100644 >>>> --- a/gcc/config/arm/arm.c >>>> +++ b/gcc/config/arm/arm.c >>>> @@ -22568,7 +22568,7 @@ arm_print_operand_punct_valid_p (unsigned char >>>> code) { >>>> >>>> return (code == '@' || code == '|' || code == '.' >>>> >>>> || code == '(' || code == ')' || code == '#' >>>> >>>> - || (TARGET_32BIT && (code == '?')) >>>> + || code == '?' >>>> >>>> || (TARGET_THUMB2 && (code == '!')) >>>> || (TARGET_THUMB && (code == '_'))); >>>> >>>> } >>>> >>>> diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md >>>> index 7e3bcb4..074b267 100644 >>>> --- a/gcc/config/arm/thumb1.md >>>> +++ b/gcc/config/arm/thumb1.md >>>> @@ -973,6 +973,92 @@ >>>> >>>> DONE; >>>> >>>> }) >>>> >>>> +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline >>>> profile, >>>> +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead >>>> leads >>>> to +;; code generation difference for ARMv6-M because the minimum length >>>> of >>>> the +;; instruction becomes 2 even for it due to a limitation in >>>> genattrtab's +;; handling of pc in the length condition. >>>> +(define_insn "thumb1_cbz" >>>> + [(set (pc) (if_then_else >>>> + (match_operator 0 "equality_operator" >>>> + [(match_operand:SI 1 "s_register_operand" "l") >>>> + (const_int 0)]) >>>> + (label_ref (match_operand 2 "" "")) >>>> + (pc)))] >>>> + "TARGET_THUMB1 && TARGET_HAVE_MOVT" >>>> +{ >>>> + if (get_attr_length (insn) == 2) >>>> + { >>>> + if (GET_CODE (operands[0]) == EQ) >>>> + return "cbz\t%1, %l2"; >>>> + else >>>> + return "cbnz\t%1, %l2"; >>>> + } >>>> + else >>>> + { >>>> + rtx t = cfun->machine->thumb1_cc_insn; >>>> + if (t != NULL_RTX) >>>> + { >>>> + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) >>>> + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) >>>> + t = NULL_RTX; >>>> + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) >>>> + { >>>> + if (!noov_comparison_operator (operands[0], VOIDmode)) >>>> + t = NULL_RTX; >>>> + } >>>> + else if (cfun->machine->thumb1_cc_mode != CCmode) >>>> + t = NULL_RTX; >>>> + } >>>> + if (t == NULL_RTX) >>>> + { >>>> + output_asm_insn ("cmp\t%1, #0", operands); >>>> + cfun->machine->thumb1_cc_insn = insn; >>>> + cfun->machine->thumb1_cc_op0 = operands[1]; >>>> + cfun->machine->thumb1_cc_op1 = operands[2]; >>>> + cfun->machine->thumb1_cc_mode = CCmode; >>>> + } >>>> + else >>>> + /* Ensure we emit the right type of condition code on the jump. */ >>>> + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, >>>> + CC_REGNUM); >>>> + >>>> + switch (get_attr_length (insn)) >>>> + { >>>> + case 4: return "b%d0\t%l2"; >>>> + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; >>>> + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; >>>> + default: gcc_unreachable (); >>>> + } >>>> + } >>>> +} >>>> + [(set (attr "far_jump") >>>> + (if_then_else >>>> + (eq_attr "length" "8") >>>> + (const_string "yes") >>>> + (const_string "no"))) >>>> + (set (attr "length") >>>> + (if_then_else >>>> + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) >>>> + (le (minus (match_dup 2) (pc)) (const_int 128)) >>>> + (not (match_test "which_alternative"))) >>>> + (const_int 2) >>>> + (if_then_else >>>> + (and (ge (minus (match_dup 2) (pc)) (const_int -250)) >>>> + (le (minus (match_dup 2) (pc)) (const_int 256))) >>>> + (const_int 4) >>>> + (if_then_else >>>> + (and (ge (minus (match_dup 2) (pc)) (const_int -2040)) >>>> + (le (minus (match_dup 2) (pc)) (const_int 2048))) >>>> + (const_int 6) >>>> + (const_int 8))))) >>>> + (set (attr "type") >>>> + (if_then_else >>>> + (eq_attr "length" "2") >>>> + (const_string "branch") >>>> + (const_string "multiple")))] >>>> +) >>>> + >>>> >>>> (define_insn "cbranchsi4_insn" >>>> >>>> [(set (pc) (if_then_else >>>> >>>> (match_operator 0 "arm_comparison_operator" >>>> >>>> Testing: >>>> >>>> * Toolchain was built successfully with and without the ARMv8-M support >>>> patches with the following multilib list: >>>> armv6-m,armv7-m,armv7e-m,cortex-m7. The code generation for crtbegin.o, >>>> crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a, >>>> libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a, >>>> libstdc++.a >>>> and libsupc++.a is unchanged for all these targets. >>>> >>>> * GCC also showed no testsuite regression when targeting ARMv8-M Baseline >>>> compared to ARMv6-M on ARM Fast Models and when targeting ARMv6-M and >>>> ARMv7-M (compared to without the patch) * GCC was bootstrapped >>>> successfully >>>> targeting Thumb-1 and targeting Thumb-2 >>>> >>>> Is this ok for stage3? >>>> >>>> Best regards, >>>> >>>> Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, ARM 6/7, ping1] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline 2016-07-12 14:57 ` Kyrill Tkachov @ 2016-07-13 10:54 ` Thomas Preudhomme 0 siblings, 0 replies; 7+ messages in thread From: Thomas Preudhomme @ 2016-07-13 10:54 UTC (permalink / raw) To: Kyrill Tkachov; +Cc: Richard Earnshaw, Ramana Radhakrishnan, gcc-patches On Tuesday 12 July 2016 15:57:41 Kyrill Tkachov wrote: > On 12/07/16 11:26, Thomas Preudhomme wrote: > > Hi Kyrill, > > Hi Thomas, > > > On Friday 20 May 2016 14:22:48 Kyrill Tkachov wrote: > >> Hi Thomas, > >> > >> On 17/05/16 11:14, Thomas Preudhomme wrote: > >>> Ping? > >>> > >>> *** gcc/ChangeLog *** > >>> > >>> 2015-11-13 Thomas Preud'homme <thomas.preudhomme@arm.com> > >>> > >>> * config/arm/arm.c (arm_print_operand_punct_valid_p): Make %? > >>> valid > >>> for Thumb-1. > >>> * config/arm/arm.h (TARGET_HAVE_CBZ): Define. > >>> (TARGET_IDIV): Set for all Thumb targets provided they have > >>> hardware > >>> divide feature. > >>> * config/arm/thumb1.md (thumb1_cbz): New insn. > >>> > >>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > >>> index > >>> f42e996e5a7ce979fe406b8261d50fb2ba005f6b..347b5b0a5cc0bc1e3b5020c8124d96 > >>> 8e > >>> 76ce48a4 100644 > >>> --- a/gcc/config/arm/arm.h > >>> +++ b/gcc/config/arm/arm.h > >>> @@ -271,9 +271,12 @@ extern void > >>> (*arm_lang_output_object_attributes_hook) > >>> (void); > >>> > >>> /* Nonzero if this chip provides the movw and movt instructions. */ > >>> #define TARGET_HAVE_MOVT (arm_arch_thumb2 || arm_arch8) > >>> > >>> +/* Nonzero if this chip provides the cb{n}z instruction. */ > >>> +#define TARGET_HAVE_CBZ (arm_arch_thumb2 || arm_arch8) > >>> + > >>> > >>> /* Nonzero if integer division instructions supported. */ > >>> #define TARGET_IDIV ((TARGET_ARM && arm_arch_arm_hwdiv) \ > >>> > >>> - || (TARGET_THUMB2 && arm_arch_thumb_hwdiv)) > >>> + || (TARGET_THUMB && arm_arch_thumb_hwdiv)) > >>> > >>> /* Nonzero if disallow volatile memory access in IT block. */ > >>> #define TARGET_NO_VOLATILE_CE (arm_arch_no_volatile_ce) > >>> > >>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > >>> index > >>> 13b4b71ac8f9c1da8ef1945f7ff6985ca59f6832..445972ce0e3fd27d4411840ff69e9e > >>> db > >>> b23994fc 100644 > >>> --- a/gcc/config/arm/arm.c > >>> +++ b/gcc/config/arm/arm.c > >>> @@ -22684,7 +22684,7 @@ arm_print_operand_punct_valid_p (unsigned char > >>> code)> > >>> > >>> { > >>> > >>> return (code == '@' || code == '|' || code == '.' > >>> > >>> || code == '(' || code == ')' || code == '#' > >>> > >>> - || (TARGET_32BIT && (code == '?')) > >>> + || code == '?' > >>> > >>> || (TARGET_THUMB2 && (code == '!')) > >>> || (TARGET_THUMB && (code == '_'))); > >>> > >>> } > >> > >> Hmm, I'm not a fan of this change. arm_print_operand_punct_valid_p is an > >> implementation of a target hook that is used to validate user-provided > >> inline asm as well and is therefore the right place to reject such > >> invalid > >> constructs. > >> > >> This is just working around the fact that the output template for the > >> [u]divsi3 patterns has a '%?' in it that is illegal in Thumb1 and will > >> not > >> be used for ARMv8-M Baseline anyway. I'd prefer it if you add a second > >> alternative to those patterns and emit the sdiv/udiv mnemonic without the > >> '%?' and enable that for the v8mb arch attribute (and mark the existing > >> alternative as requiring the "32" arch attribute). > >> > >>> diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md > >>> index > >>> 4572456b8bc98503061846cad94bc642943db3a2..1b01ef6ce731fe3ff37c3d8c048fb9 > >>> d5 > >>> e7829b35 100644 > >>> --- a/gcc/config/arm/thumb1.md > >>> +++ b/gcc/config/arm/thumb1.md > >>> @@ -973,6 +973,92 @@ > >>> > >>> DONE; > >>> > >>> }) > >>> > >>> +;; A pattern for the cb(n)z instruction added in ARMv8-M baseline > >>> profile, > >>> +;; adapted from cbranchsi4_insn. Modifying cbranchsi4_insn instead > >>> leads > >>> to +;; code generation difference for ARMv6-M because the minimum length > >>> of the +;; instruction becomes 2 even for it due to a limitation in > >>> genattrtab's +;; handling of pc in the length condition. > >>> +(define_insn "thumb1_cbz" > >>> + [(set (pc) (if_then_else > >>> + (match_operator 0 "equality_operator" > >>> + [(match_operand:SI 1 "s_register_operand" "l") > >>> + (const_int 0)]) > >>> + (label_ref (match_operand 2 "" "")) > >>> + (pc)))] > >>> + "TARGET_THUMB1 && TARGET_HAVE_MOVT" > >>> +{ > >> > >> s/TARGET_HAVE_MOVT/TARGET_HAVE_CBZ/ > >> > >>> + if (get_attr_length (insn) == 2) > >>> + { > >>> + if (GET_CODE (operands[0]) == EQ) > >>> + return "cbz\t%1, %l2"; > >>> + else > >>> + return "cbnz\t%1, %l2"; > >>> + } > >>> + else > >>> + { > >>> + rtx t = cfun->machine->thumb1_cc_insn; > >>> + if (t != NULL_RTX) > >>> + { > >>> + if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) > >>> + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) > >>> + t = NULL_RTX; > >>> + if (cfun->machine->thumb1_cc_mode == CC_NOOVmode) > >>> + { > >>> + if (!noov_comparison_operator (operands[0], VOIDmode)) > >>> + t = NULL_RTX; > >>> + } > >>> + else if (cfun->machine->thumb1_cc_mode != CCmode) > >>> + t = NULL_RTX; > >>> + } > >>> + if (t == NULL_RTX) > >>> + { > >>> + output_asm_insn ("cmp\t%1, #0", operands); > >>> + cfun->machine->thumb1_cc_insn = insn; > >>> + cfun->machine->thumb1_cc_op0 = operands[1]; > >>> + cfun->machine->thumb1_cc_op1 = operands[2]; > >>> + cfun->machine->thumb1_cc_mode = CCmode; > >>> + } > >>> + else > >>> + /* Ensure we emit the right type of condition code on the jump. */ > >>> + XEXP (operands[0], 0) = gen_rtx_REG (cfun->machine->thumb1_cc_mode, > >>> + CC_REGNUM); > >>> + > >>> + switch (get_attr_length (insn)) > >>> + { > >>> + case 4: return "b%d0\t%l2"; > >>> + case 6: return "b%D0\t.LCB%=;b\t%l2\t%@long jump\n.LCB%=:"; > >>> + case 8: return "b%D0\t.LCB%=;bl\t%l2\t%@far jump\n.LCB%=:"; > >>> + default: gcc_unreachable (); > >>> + } > >>> + } > >>> +} > >>> + [(set (attr "far_jump") > >>> + (if_then_else > >>> + (eq_attr "length" "8") > >>> + (const_string "yes") > >>> + (const_string "no"))) > >>> + (set (attr "length") > >>> + (if_then_else > >>> + (and (ge (minus (match_dup 2) (pc)) (const_int 2)) > >>> + (le (minus (match_dup 2) (pc)) (const_int 128)) > >>> + (not (match_test "which_alternative"))) > >> > >> This pattern only has one alternative so "which_alternative" > >> will always be 0, so the (not (match_test "which_alternative")) > >> test inside the 'and' is redundant and can be removed. > > > > What about the attached updated patch? > > This is ok with one ChangeLog nit. Done as per your suggestion. Best regards, Thomas ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-07-13 10:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-17 8:18 [PATCH, ARM 6/6] Add support for CB(N)Z and (U|S)DIV to ARMv8-M Baseline Thomas Preud'homme 2016-05-17 10:14 ` [PATCH, ARM 6/7, ping1] " Thomas Preudhomme 2016-05-20 13:23 ` Kyrill Tkachov 2016-07-06 16:55 ` Thomas Preudhomme 2016-07-12 10:27 ` Thomas Preudhomme 2016-07-12 14:57 ` Kyrill Tkachov 2016-07-13 10:54 ` Thomas Preudhomme
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).