RE: [PATCH ARM]Extend thumb1_reorg to save more comparison instructions

["bin.cheng" <bin dot cheng at arm dot com>]

> -----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

Attachment: thumb1_reorg-20130917.txt
Description: Text document