Patchwork [ARM] Extend thumb1_reorg to save more comparison instructions

login
register
mail settings
Submitter Bin Cheng
Date April 18, 2013, 5:34 a.m.
Message ID <001b01ce3bf6$73abb6f0$5b0324d0$@cheng@arm.com>
Download mbox | patch
Permalink /patch/237435/
State New
Headers show

Comments

Bin Cheng - April 18, 2013, 5:34 a.m.
Hi,
Before thumb1_reorg, ARM backend uses peephole to save comparison
instructions when a flag setting move is found before branch instruction.
Since we are using thumb1_reog now, it can be extended to catch more
opportunities by searching flag setting move instruction before branch,
rather than only the exact one before branch. 
For example:

mov r0, r1
//other insns does not kill r0
branch if (r0 == 0)
//other insns

Tested on thumb1, is it OK?


2013-04-18  Bin Cheng  <bin.cheng@arm.com>

	* config/arm/arm.c (thumb1_reorg): Search for flag setting insn
	before branch in same basic block.  Check both src and dest of
	the move insn.
Richard Earnshaw - Sept. 12, 2013, 3:24 p.m.
On 18/04/13 06:34, Bin Cheng wrote:
> Hi,
> Before thumb1_reorg, ARM backend uses peephole to save comparison
> instructions when a flag setting move is found before branch instruction.
> Since we are using thumb1_reog now, it can be extended to catch more
> opportunities by searching flag setting move instruction before branch,
> rather than only the exact one before branch. 
> For example:
> 
> mov r0, r1
> //other insns does not kill r0
> branch if (r0 == 0)
> //other insns
> 
> Tested on thumb1, is it OK?
> 
> 
> 2013-04-18  Bin Cheng  <bin.cheng@arm.com>
> 
> 	* config/arm/arm.c (thumb1_reorg): Search for flag setting insn
> 	before branch in same basic block.  Check both src and dest of
> 	the move insn.
> 
> 

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.

> @@ -14050,12 +14068,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);
> 

R.

Patch

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
+		    && 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);
       if (!set)
 	continue;
@@ -14050,12 +14068,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);