> -----Original Message-----
> From: Richard Earnshaw
> Sent: Thursday, September 12, 2013 11:24 PM
> To: Bin Cheng
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH ARM]Extend thumb1_reorg to save more comparison
> instructions
>
> On 18/04/13 06:34, Bin Cheng wrote:
>
> Sorry for the delay, I've been trying to get my head around this one.
>
> > thumb1_reorg-20130417.txt
> >
> >
> > Index: gcc/config/arm/arm.c
> >
> ==========================================================
> =========
> > --- gcc/config/arm/arm.c (revision 197562)
> > +++ gcc/config/arm/arm.c (working copy)
> > @@ -14026,6 +14026,7 @@ thumb1_reorg (void)
> > rtx set, dest, src;
> > rtx pat, op0;
> > rtx prev, insn = BB_END (bb);
> > + bool insn_clobbered = false;
> >
> > while (insn != BB_HEAD (bb) && DEBUG_INSN_P (insn))
> > insn = PREV_INSN (insn);
> > @@ -14034,12 +14035,29 @@ thumb1_reorg (void)
> > if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn)
> > continue;
> >
> > - /* Find the first non-note insn before INSN in basic block BB.
*/
> > + /* Get the register with which we are comparing. */
> > + pat = PATTERN (insn);
> > + op0 = XEXP (XEXP (SET_SRC (pat), 0), 0);
> > +
> > + /* Find the first flag setting 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);
> > + for (prev = PREV_INSN (insn);
> > + (!insn_clobbered
> > + && prev != BB_HEAD (bb)
> > + && (NOTE_P (prev)
> > + || DEBUG_INSN_P (prev)
> > + || (GET_CODE (prev) == SET
>
> This can't be right. prev is an insn of some form, so the test that it is
a SET will
> always fail.
>
> What you need to do here is to initialize 'set' to null before the loop
and then
> have something like
>
> || ((set = single_set (prev)) != NULL
>
> > + && get_attr_conds (prev) == CONDS_NOCOND)));
> > + prev = PREV_INSN (prev))
> > + {
> > + if (reg_set_p (op0, prev))
> > + insn_clobbered = true;
> > + }
> >
> > + /* Skip if op0 is clobbered by insn other than prev. */
> > + if (insn_clobbered)
> > + continue;
> > +
> > set = single_set (prev);
>
> This now becomes redundant and ...
>
> > if (!set)
> > continue;
>
> This will be based on the set you extracted above.
>
Hi Richard, here is the updated patch according to your comments. Tested on
thumb1, please review.
Thanks.
bin
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c (revision 202599)
+++ gcc/config/arm/arm.c (working copy)
@@ -14265,9 +14265,10 @@ thumb1_reorg (void)
FOR_EACH_BB (bb)
{
- rtx set, dest, src;
- rtx pat, op0;
+ rtx dest, src;
+ rtx pat, op0, set = NULL;
rtx prev, insn = BB_END (bb);
+ bool insn_clobbered = false;
while (insn != BB_HEAD (bb) && DEBUG_INSN_P (insn))
insn = PREV_INSN (insn);
@@ -14276,13 +14277,29 @@ thumb1_reorg (void)
if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn)
continue;
- /* Find the first non-note insn before INSN in basic block BB. */
+ /* Get the register with which we are comparing. */
+ pat = PATTERN (insn);
+ op0 = XEXP (XEXP (SET_SRC (pat), 0), 0);
+
+ /* Find the first flag setting 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);
+ for (prev = PREV_INSN (insn);
+ (!insn_clobbered
+ && prev != BB_HEAD (bb)
+ && (NOTE_P (prev)
+ || DEBUG_INSN_P (prev)
+ || ((set = single_set (prev)) != NULL
+ && get_attr_conds (prev) == CONDS_NOCOND)));
+ prev = PREV_INSN (prev))
+ {
+ if (reg_set_p (op0, prev))
+ insn_clobbered = true;
+ }
- set = single_set (prev);
+ /* Skip if op0 is clobbered by insn other than prev. */
+ if (insn_clobbered)
+ continue;
+
if (!set)
continue;
@@ -14292,12 +14309,9 @@ thumb1_reorg (void)
|| !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))
+ in INSN. Both src and dest of the move insn are checked. */
+ if (REGNO (op0) == REGNO (src) || REGNO (op0) == REGNO (dest))
{
dest = copy_rtx (dest);
src = copy_rtx (src);
