Re: [PATCH: PR target/40835] Remove comparison with 0 after instruction movs

[Carrot Wei <carrot at google dot com>]

Could anybody help to review this patch?

thanks
Wei Guozhi

On Tue, Jul 28, 2009 at 4:25 PM, Carrot Wei <carrot@google.com> wrote:
> In thumb mode the cmp instruction in the following sequence can be removed
> since the previous movs instruction has already set the condition code
> according to the value of r1.
>
> ? ? ? movs r0, r1
> ? ? ? cmp ?r1, 0
> ? ? ? beq ?.L3
>
> In thumb mode we don't have insn patterns to express compare and conditional
> branch separately, so this optimization is implemented as a peephole rule.
>
> Test:
> This patch was applied to trunk GCC and tested on the arm emulator with newlib.
> No new failure.
>
> ChangeLog:
> 2009-07-28 ?Wei Guozhi ?<carrot@google.com>
>
> ? ? ? ?PR target/40835
> ? ? ? ?* config/arm/arm.c (emit_branch_after_movs): New function.
> ? ? ? ?(removable_cmp_0): New function.
> ? ? ? ?* config/arm/arm-protos.h (emit_branch_after_movs): Declare it.
> ? ? ? ?(removable_cmp_0): Declare it.
> ? ? ? ?* config/arm/arm.md: Add peephole rule to do the optimization.
>
> 2009-07-28 ?Wei Guozhi ?<carrot@google.com>
>
> ? ? ? ?PR target/40835
> ? ? ? ?* gcc.target/arm/pr40835: New testcase.
>
>
> thanks
> Carrot
>
>
> Index: arm-protos.h
> ===================================================================
> --- arm-protos.h ? ? ? ?(revision 150037)
> +++ arm-protos.h ? ? ? ?(working copy)
> @@ -146,7 +146,8 @@ extern const char *vfp_output_fstmd (rtx
> ?extern void arm_set_return_address (rtx, rtx);
> ?extern int arm_eliminable_register (rtx);
> ?extern const char *arm_output_shift(rtx *, int);
> -
> +extern bool removable_cmp_0 (rtx cmp_op);
> +extern const char *emit_branch_after_movs (rtx *operands);
> ?extern bool arm_output_addr_const_extra (FILE *, rtx);
>
> ?#if defined TREE_CODE
>
>
> Index: arm.c
> ===================================================================
> --- arm.c ? ? ? (revision 150037)
> +++ arm.c ? ? ? (working copy)
> @@ -20038,4 +20038,62 @@ arm_frame_pointer_required (void)
> ? ? ? ? ? || (TARGET_ARM && TARGET_APCS_FRAME && ! leaf_function_p ()));
> ?}
>
> +/* In thumb mode the cmp instruction in the following sequence can be
> + ? removed since the previous movs instruction has already set the
> + ? condition code according to the value of r1.
> + ? ? ? movs r0, r1
> + ? ? ? cmp ?r1, 0
> + ? ? ? beq ?.L3 ? ? ?*/
> +
> +const char *
> +emit_branch_after_movs (rtx *operands)
> +{
> + ?char buf[100];
> +
> + ?/* operands[2] is the compare rtx. ?*/
> + ?switch (GET_CODE (operands[2]))
> + ? ?{
> + ? ? ?case EQ:
> + ? ? ? ?sprintf (buf, "beq");
> + ? ? ? ?break;
> +
> + ? ? ?case NE:
> + ? ? ? ?sprintf (buf, "bne");
> + ? ? ? ?break;
> +
> + ? ? ?case LT:
> + ? ? ? ?sprintf (buf, "bmi");
> + ? ? ? ?break;
> +
> + ? ? ?case GE:
> + ? ? ? ?sprintf (buf, "bpl");
> + ? ? ? ?break;
> +
> + ? ? ?default:
> + ? ? ? ?gcc_unreachable ();
> + ? ?}
> +
> + ?output_asm_insn ("movs\t%0, %1", operands);
> + ?strcat (buf, "\t%l3");
> + ?output_asm_insn (buf, operands);
> + ?return "";
> +}
> +
> +/* If the second operand is 0 the following compare rtx codes depend on N
> + ? and Z flags only, which have been set by previous movs instruction. So
> + ? can be removed. ?*/
> +bool
> +removable_cmp_0 (rtx cmp_op)
> +{
> + ?switch (GET_CODE (cmp_op))
> + ? ?{
> + ? ? ?case EQ:
> + ? ? ?case NE:
> + ? ? ?case LT:
> + ? ? ?case GE:
> + ? ? ? ?return true;
> + ? ?}
> +
> + ?return false;
> +}
> +
> ?#include "gt-arm.h"
>
>
> Index: arm.md
> ===================================================================
> --- arm.md ? ? ?(revision 150037)
> +++ arm.md ? ? ?(working copy)
> @@ -7707,6 +7707,28 @@
> ? ? ? ? ? ? ? ?(const_int 8))))]
> ?)
>
> +;; In thumb mode the cmp instruction in the following sequence can be
> +;; removed since the previous movs instruction has already set the
> +;; condition code according to the value of r1.
> +;; ? ?movs r0, r1
> +;; ? ?cmp ?r1, 0
> +;; ? ?beq ?.L3
> +
> +(define_peephole
> + ?[(set (match_operand:SI 0 "low_register_operand" "")
> + ? ? ? ?(match_operand:SI 1 "low_register_operand" ""))
> + ? (set (pc)
> + ? ? ? ?(if_then_else (match_operator 2 "arm_comparison_operator"
> + ? ? ? ? ? ? ? ? ? ? ? [(match_dup 1) (const_int 0)])
> + ? ? ? ? ? ? ? ? ? ? ?(label_ref (match_operand 3 "" ""))
> + ? ? ? ? ? ? ? ? ? ? ?(pc)))]
> + ?"TARGET_THUMB && (get_attr_length (insn) == 4)
> + ? && removable_cmp_0 (operands[2])"
> + ?"*
> + ?return emit_branch_after_movs (operands);
> + ?"
> +)
> +
> ?;; Comparison and test insns
>
> ?(define_insn "*arm_cmpsi_insn"
>
>
> Index: pr40835.c
> ===================================================================
> --- pr40835.c ? (revision 0)
> +++ pr40835.c ? (revision 0)
> @@ -0,0 +1,39 @@
> +/* { dg-options "-mthumb -Os -march=armv5te" } ?*/
> +/* { dg-final { scan-assembler-not "cmp" } } */
> +
> +int bar();
> +void goo(int, int);
> +
> +void eq()
> +{
> + ?int v = bar();
> + ?if (v == 0)
> + ? ?return;
> + ?goo(1, v);
> +}
> +
> +void ge()
> +{
> + ?int v = bar();
> + ?if (v >= 0)
> + ? ?return;
> + ?goo(1, v);
> +}
> +
> +void lt()
> +{
> + ?int v = bar();
> + ?if (v < 0)
> + ? ?return;
> + ?goo(1, v);
> +}
> +
> +unsigned int foo();
> +
> +void leu()
> +{
> + ?unsigned int v = foo();
> + ?if (v <= 0)
> + ? ?return;
> + ?goo(1, v);
> +}
>