> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandif...@googlemail.com]
> Sent: Wednesday, September 05, 2012 6:09 AM
> To: Bin Cheng
> Cc: Ramana Radhakrishnan; 'Eric Botcazou'; gcc-patches@gcc.gnu.org
> Subject: Re: Ping: [PATCH GCC/ARM] Fix problem that hardreg_cprop
> opportunities are missed on thumb1
> Subtraction of zero isn't canonical rtl though. Passes after peephole2
would
> be well within their rights to simplify the expression back to a move.
> From that point of view, making the passes recognise (plus X 0) and (minus
X 0)
> as special cases would be inconsistent.
>
> Rather than make the Thumb 1 CC usage implicit in the rtl stream, and
carry
> the current state around in cfun->machine, it seems like it would be
better to
> get md_reorg to rewrite the instructions into a form that makes the use of
> condition codes explicit.
>
> md_reorg also sounds like a better place in the pipeline than peephole2 to
be
> doing this kind of transformation, although I admit I have zero evidence
to
> back that up...
>
Hi Richard,
This is the updated patch according to your suggestions. I removed the
peephole2 patterns and introduced function thumb1_reorg to rewrite
instructions in md_reorg pass.
In addition to missed propagation, this patch also detects following case:
mov r5, r0
str r0, [r4] <-------miscellaneous irrelevant instructions
[cmp r0, 0] <-------saved
bne .Lxxx
Patch tested on arm-none-eabi/cortex-m0, no regressions introduced.
Is it OK?
Thanks.
2012-09-25 Bin Cheng <bin.ch...@arm.com>
* config/arm/arm.c (thumb1_reorg): New function.
(arm_reorg): Call thumb1_reorg.
(thumb1_final_prescan_insn): Record src operand in thumb1_cc_op0.
* config/arm/arm.md : Remove peephole2 patterns which rewrites move
into subtract of ZERO.
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c (revision 191088)
+++ gcc/config/arm/arm.c (working copy)
@@ -13263,6 +13263,62 @@ note_invalid_constants (rtx insn, HOST_WIDE_INT ad
return;
}
+/* Rewrite move insn into subtract of 0 if the condition codes will
+ be useful in next conditional jump insn. */
+
+static void
+thumb1_reorg (void)
+{
+ basic_block bb;
+
+ FOR_EACH_BB (bb)
+ {
+ rtx set, dest, src;
+ rtx pat, op0;
+ rtx prev, insn = BB_END (bb);
+
+ while (insn != BB_HEAD (bb) && DEBUG_INSN_P (insn))
+ insn = PREV_INSN (insn);
+
+ /* Find the last cbranchsi4_insn in basic block BB. */
+ if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn)
+ continue;
+
+ /* Find the first non-note insn before INSN in basic block BB. */
+ gcc_assert (insn != BB_HEAD (bb));
+ prev = PREV_INSN (insn);
+ while (prev != BB_HEAD (bb) && (NOTE_P (prev) || DEBUG_INSN_P (prev)))
+ prev = PREV_INSN (prev);
+
+ set = single_set (prev);
+ if (!set)
+ continue;
+
+ dest = SET_DEST (set);
+ src = SET_SRC (set);
+ if (!low_register_operand (dest, SImode)
+ || !low_register_operand (src, SImode))
+ continue;
+
+ pat = PATTERN (insn);
+ op0 = XEXP (XEXP (SET_SRC (pat), 0), 0);
+ /* Rewrite move into subtract of 0 if its operand is compared with ZERO
+ in INSN. Don't need to check dest since cprop_hardreg pass propagates
+ src into INSN. */
+ if (REGNO (op0) == REGNO (src))
+ {
+ dest = copy_rtx (dest);
+ src = copy_rtx (src);
+ src = gen_rtx_MINUS (SImode, src, const0_rtx);
+ PATTERN (prev) = gen_rtx_SET (VOIDmode, dest, src);
+ INSN_CODE (prev) = -1;
+ /* Set test register in INSN to dest. */
+ XEXP (XEXP (SET_SRC (pat), 0), 0) = copy_rtx (dest);
+ INSN_CODE (insn) = -1;
+ }
+ }
+}
+
/* Convert instructions to their cc-clobbering variant if possible, since
that allows us to use smaller encodings. */
@@ -13459,7 +13515,9 @@ arm_reorg (void)
HOST_WIDE_INT address = 0;
Mfix * fix;
- if (TARGET_THUMB2)
+ if (TARGET_THUMB1)
+ thumb1_reorg ();
+ else if (TARGET_THUMB2)
thumb2_reorg ();
/* Ensure all insns that must be split have been split at this point.
@@ -21736,6 +21794,12 @@ thumb1_final_prescan_insn (rtx insn)
if (src1 == const0_rtx)
cfun->machine->thumb1_cc_mode = CCmode;
}
+ else if (REG_P (SET_DEST (set)) && REG_P (SET_SRC (set)))
+ {
+ /* Record the src register operand instead of dest because
+ cprop_hardreg pass propagates src. */
+ cfun->machine->thumb1_cc_op0 = SET_SRC (set);
+ }
}
else if (conds != CONDS_NOCOND)
cfun->machine->thumb1_cc_insn = NULL_RTX;
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md (revision 191088)
+++ gcc/config/arm/arm.md (working copy)
@@ -7102,43 +7102,6 @@
(const_int 8))))]
)
-;; Two peepholes to generate subtract of 0 instead of a move if the
-;; condition codes will be useful.
-(define_peephole2
- [(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_THUMB1"
- [(set (match_dup 0) (minus:SI (match_dup 1) (const_int 0)))
- (set (pc)
- (if_then_else (match_op_dup 2 [(match_dup 0) (const_int 0)])
- (label_ref (match_dup 3))
- (pc)))]
- "")
-
-;; Sigh! This variant shouldn't be needed, but combine often fails to
-;; merge cases like this because the op1 is a hard register in
-;; arm_class_likely_spilled_p.
-(define_peephole2
- [(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 0) (const_int 0)])
- (label_ref (match_operand 3 "" ""))
- (pc)))]
- "TARGET_THUMB1"
- [(set (match_dup 0) (minus:SI (match_dup 1) (const_int 0)))
- (set (pc)
- (if_then_else (match_op_dup 2 [(match_dup 0) (const_int 0)])
- (label_ref (match_dup 3))
- (pc)))]
- "")
-
(define_insn "*negated_cbranchsi4"
[(set (pc)
(if_then_else
