* [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).