Patchwork [SH] PR 51244 - Add combiner patterns for branch inversion

login
register
mail settings
Submitter Oleg Endo
Date Aug. 12, 2012, 12:47 p.m.
Message ID <1344775650.2279.23.camel@yam-132-YW-E178-FTW>
Download mbox | patch
Permalink /patch/176782/
State New
Headers show

Comments

Oleg Endo - Aug. 12, 2012, 12:47 p.m.
Hello,

This adds some patters that allow the combine pass to catch inverted
compare and branch opportunities better.

Tested on rev 190273 with
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

and no new failures.
OK?

PS: I'm still puzzled why combine sometimes takes different 'routes' on
SH4A and/or SH2A which requires extra patterns to handle some cases ... 

Cheers,
Oleg

ChangeLog:

	PR target/51244
	* config/sh/sh.md: Add splits for inverted compare and branch 
	opportunities.
	(*cmpeqsi_t): New insn.
	(cmpgtsi_t, cmpgesi_t): Swap r and N alternatives.
	(cmpgeusi_t): Use satisfies_constraint_Z.  Emit sett insn in 
	replacement insn list and not in the preparation statements.
	(clrt, sett): Add mt_group attribute.

testsuite/ChangeLog:

	PR target/51244
	* gcc.target/sh/pr51244-7.c: New.
	* gcc.target/sh/pr51244-8.c: New.
	* gcc.target/sh/pr51244-9.c: New.
	* gcc.target/sh/pr51244-10.c: New.
Kaz Kojima - Aug. 12, 2012, 9:29 p.m.
Oleg Endo <oleg.endo@t-online.de> wrote:
> This adds some patters that allow the combine pass to catch inverted
> compare and branch opportunities better.
> 
> Tested on rev 190273 with
> make -k check RUNTESTFLAGS="--target_board=sh-sim
> \{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"
> 
> and no new failures.
> OK?

OK.

Regards,
	kaz

Patch

Index: gcc/testsuite/gcc.target/sh/pr51244-8.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr51244-8.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-8.c	(revision 0)
@@ -0,0 +1,27 @@ 
+/* Check that compare-branch is inverted properly.
+   Example:
+	mov	#1,r0	->	tst	r8,r8
+	neg	r8,r1		bt	.L47
+	shad	r1,r0
+	tst	#1,r0
+	bf	.L47
+*/
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*" } { "" } } */
+/* { dg-final { scan-assembler-not "shad|neg" } } */
+
+int test_01_00 (int*, void*);
+int
+test_01 (int* m, void* v)
+{
+  unsigned long n = (unsigned long)v - 1;
+
+  if (!n)
+    return 50;
+  
+  if (1 & (1 << n))	/* if n == 0: 1 & (1 << 0) -> true  */
+    return 60;		
+  else			/* if n != 0: 1 & (1 << n) -> false  */
+    return -8;
+}
Index: gcc/testsuite/gcc.target/sh/pr51244-7.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr51244-7.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-7.c	(revision 0)
@@ -0,0 +1,26 @@ 
+/* Check that compare-branch is inverted properly.
+   Example:
+	clrt		->	clrt
+	subc	r0,r6		subc	r0,r6
+	mov	r3,r7		mov	r3,r7
+	subc	r1,r7		subc	r1,r7
+	mov	#0,r1		tst	r7,r7
+	cmp/hi	r1,r7		bf	.L111
+	bt	.L111		bra	.L197
+	bra	.L197
+	nop
+*/
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*" } { "" } } */
+/* { dg-final { scan-assembler-not "cmp/hi" } } */
+/* { dg-final { scan-assembler-not "mov\t#0" } } */
+
+int other_func (long long);
+int
+test_00 (unsigned long long a, unsigned long long b)
+{
+  if ((a - b) > 0xFFFFFFFFLL)
+    return other_func (a - b);
+  return 20;
+}
Index: gcc/testsuite/gcc.target/sh/pr51244-9.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr51244-9.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-9.c	(revision 0)
@@ -0,0 +1,35 @@ 
+/* Check that compare-branch is inverted properly.
+   Example:
+	mov.w	.L566,r2	->	mov.w	.L566,r2
+	add	r11,r2			add	r11,r2
+	mov.l	@(12,r2),r7		mov.l	@(8,r2),r5
+	mov.l	@(8,r2),r5		mov.l	@(12,r2),r2
+	mov	#0,r2			tst	r2,r2
+	cmp/hi	r2,r7			bt	.L534
+	bf	.L534
+*/
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*" } { "" } } */
+/* { dg-final { scan-assembler-not "mov\t#0" } } */
+static inline unsigned int
+test_03_00 (unsigned int x)
+{
+  /* Return unassigned value on purpose.  */
+  unsigned int res;
+  return res;
+}
+
+struct S
+{
+  unsigned int a;
+  unsigned int b;
+};
+
+int test_03 (struct S* i)
+{
+ if ((i->a != 2 && i->a != 3) || i->a > test_03_00 (i->b))
+   return -5;
+
+ return -55;
+}
Index: gcc/testsuite/gcc.target/sh/pr51244-10.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr51244-10.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-10.c	(revision 0)
@@ -0,0 +1,27 @@ 
+/* Check that compare-branch is inverted properly.
+   In this case the improved bit test is a side effect of compare-branch
+   inversion patterns, even though the branch condition does not get
+   inverted here.
+   Example:
+	mov.b	@(14,r9),r0	->	mov.b	@(14,r9),r0
+	shll	r0			cmp/pz	r0
+	subc	r0,r0			bt	.L192
+	and	#1,r0
+	tst	r0,r0
+	bt	.L195
+*/
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*" } { "" } } */
+/* { dg-final { scan-assembler-not "shll|subc|and" } } */
+int
+test_00 (int* p)
+{
+  int nr = 15;
+  volatile char* addr = (volatile char*)&p[1];
+
+  if ((addr[(nr >> 3) ^ 7] & (1 << (nr & 7))) == 0)
+    return 40;
+  else
+    return 50;
+}
Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md	(revision 190273)
+++ gcc/config/sh/sh.md	(working copy)
@@ -742,12 +742,6 @@ 
 }
   [(set_attr "type" "mt_group")])
 
-;; ??? Perhaps should only accept reg/constant if the register is reg 0.
-;; That would still allow reload to create cmpi instructions, but would
-;; perhaps allow forcing the constant into a register when that is better.
-;; Probably should use r0 for mem/imm compares, but force constant into a
-;; register for pseudo/imm compares.
-
 (define_insn "cmpeqsi_t"
   [(set (reg:SI T_REG)
 	(eq:SI (match_operand:SI 0 "arith_reg_operand" "r,z,r")
@@ -759,24 +753,40 @@ 
 	cmp/eq	%1,%0"
    [(set_attr "type" "mt_group")])
 
+;; FIXME: For some reason, on SH4A and SH2A combine fails to simplify this
+;; pattern by itself.  What this actually does is:
+;;	x == 0: (1 >> 0-0) & 1 = 1
+;;	x != 0: (1 >> 0-x) & 1 = 0
+;; Without this the test pr51244-8.c fails on SH2A and SH4A.
+(define_insn_and_split "*cmpeqsi_t"
+  [(set (reg:SI T_REG)
+	(and:SI (lshiftrt:SI
+		  (const_int 1)
+		  (neg:SI (match_operand:SI 0 "arith_reg_operand" "r")))
+		(const_int 1)))]
+  "TARGET_SH1"
+  "#"
+  "&& 1"
+  [(set (reg:SI T_REG) (eq:SI (match_dup 0) (const_int 0)))])
+
 (define_insn "cmpgtsi_t"
   [(set (reg:SI T_REG)
 	(gt:SI (match_operand:SI 0 "arith_reg_operand" "r,r")
-	       (match_operand:SI 1 "arith_reg_or_0_operand" "r,N")))]
+	       (match_operand:SI 1 "arith_reg_or_0_operand" "N,r")))]
   "TARGET_SH1"
   "@
-	cmp/gt	%1,%0
-	cmp/pl	%0"
+	cmp/pl	%0
+	cmp/gt	%1,%0"
    [(set_attr "type" "mt_group")])
 
 (define_insn "cmpgesi_t"
   [(set (reg:SI T_REG)
 	(ge:SI (match_operand:SI 0 "arith_reg_operand" "r,r")
-	       (match_operand:SI 1 "arith_reg_or_0_operand" "r,N")))]
+	       (match_operand:SI 1 "arith_reg_or_0_operand" "N,r")))]
   "TARGET_SH1"
   "@
-	cmp/ge	%1,%0
-	cmp/pz	%0"
+	cmp/pz	%0
+	cmp/ge	%1,%0"
    [(set_attr "type" "mt_group")])
 
 ;; FIXME: This is actually wrong.  There is no way to literally move a
@@ -815,6 +825,99 @@ 
   DONE;
 })
 
+;; Combine patterns to invert compare and branch operations for which we
+;; don't have actual comparison insns.  These patterns are used in cases
+;; which appear after the initial cbranchsi expansion, which also does
+;; some condition inversion.
+
+(define_split
+  [(set (pc)
+	(if_then_else (ne (match_operand:SI 0 "arith_reg_operand" "")
+			  (match_operand:SI 1 "arith_reg_or_0_operand" ""))
+		      (label_ref (match_operand 2))
+		      (pc)))
+   (clobber (reg:SI T_REG))]
+  "TARGET_SH1"
+  [(set (reg:SI T_REG) (eq:SI (match_dup 0) (match_dup 1)))
+   (set (pc) (if_then_else (eq (reg:SI T_REG) (const_int 0))
+			   (label_ref (match_dup 2))
+			   (pc)))])
+
+;; FIXME: Similar to the *cmpeqsi_t pattern above, for some reason, on SH4A
+;; and SH2A combine fails to simplify this pattern by itself.
+;; What this actually does is:
+;;	x == 0: (1 >> 0-0) & 1 = 1
+;;	x != 0: (1 >> 0-x) & 1 = 0
+;; Without this the test pr51244-8.c fails on SH2A and SH4A.
+(define_split
+  [(set (pc)
+	(if_then_else
+	  (eq (and:SI (lshiftrt:SI
+			(const_int 1)
+			(neg:SI (match_operand:SI 0 "arith_reg_operand" "")))
+		      (const_int 1))
+	      (const_int 0))
+	  (label_ref (match_operand 2))
+	  (pc)))
+   (clobber (reg:SI T_REG))]
+  "TARGET_SH1"
+  [(set (reg:SI T_REG) (eq:SI (match_dup 0) (const_int 0)))
+   (set (pc) (if_then_else (eq (reg:SI T_REG) (const_int 0))
+			   (label_ref (match_dup 2))
+			   (pc)))])
+
+(define_split
+  [(set (pc)
+	(if_then_else (le (match_operand:SI 0 "arith_reg_operand" "")
+			  (match_operand:SI 1 "arith_reg_or_0_operand" ""))
+		      (label_ref (match_operand 2))
+		      (pc)))
+   (clobber (reg:SI T_REG))]
+  "TARGET_SH1"
+  [(set (reg:SI T_REG) (gt:SI (match_dup 0) (match_dup 1)))
+   (set (pc) (if_then_else (eq (reg:SI T_REG) (const_int 0))
+			   (label_ref (match_dup 2))
+			   (pc)))])
+
+(define_split
+  [(set (pc)
+	(if_then_else (lt (match_operand:SI 0 "arith_reg_operand" "")
+			  (match_operand:SI 1 "arith_reg_or_0_operand" ""))
+		      (label_ref (match_operand 2))
+		      (pc)))
+   (clobber (reg:SI T_REG))]
+  "TARGET_SH1"
+  [(set (reg:SI T_REG) (ge:SI (match_dup 0) (match_dup 1)))
+   (set (pc) (if_then_else (eq (reg:SI T_REG) (const_int 0))
+			   (label_ref (match_dup 2))
+			   (pc)))])
+
+(define_split
+  [(set (pc)
+	(if_then_else (leu (match_operand:SI 0 "arith_reg_operand" "")
+			   (match_operand:SI 1 "arith_reg_operand" ""))
+		      (label_ref (match_operand 2))
+		      (pc)))
+   (clobber (reg:SI T_REG))]
+  "TARGET_SH1"
+  [(set (reg:SI T_REG) (gtu:SI (match_dup 0) (match_dup 1)))
+   (set (pc) (if_then_else (eq (reg:SI T_REG) (const_int 0))
+			   (label_ref (match_dup 2))
+			   (pc)))])
+
+(define_split
+  [(set (pc)
+	(if_then_else (ltu (match_operand:SI 0 "arith_reg_operand" "")
+			   (match_operand:SI 1 "arith_reg_operand" ""))
+		      (label_ref (match_operand 2))
+		      (pc)))
+   (clobber (reg:SI T_REG))]
+  "TARGET_SH1"
+  [(set (reg:SI T_REG) (geu:SI (match_dup 0) (match_dup 1)))
+   (set (pc) (if_then_else (eq (reg:SI T_REG) (const_int 0))
+			   (label_ref (match_dup 2))
+			   (pc)))])
+
 ;; -------------------------------------------------------------------------
 ;; SImode unsigned integer comparisons
 ;; -------------------------------------------------------------------------
@@ -825,13 +928,10 @@ 
 		(match_operand:SI 1 "arith_reg_or_0_operand" "rN")))]
   "TARGET_SH1"
   "cmp/hs	%1,%0"
-  "&& operands[1] == CONST0_RTX (SImode)"
-  [(pc)]
-{
-  emit_insn (gen_sett ());
-  DONE;
-}
-   [(set_attr "type" "mt_group")])
+  "&& satisfies_constraint_Z (operands[0])"
+  [(set (reg:SI T_REG) (const_int 1))]
+  ""
+  [(set_attr "type" "mt_group")])
 
 (define_insn "cmpgtusi_t"
   [(set (reg:SI T_REG)
@@ -839,7 +939,7 @@ 
 		(match_operand:SI 1 "arith_reg_operand" "r")))]
   "TARGET_SH1"
   "cmp/hi	%1,%0"
-   [(set_attr "type" "mt_group")])
+  [(set_attr "type" "mt_group")])
 
 
 ;; -------------------------------------------------------------------------
@@ -5132,14 +5232,15 @@ 
 (define_insn "clrt"
   [(set (reg:SI T_REG) (const_int 0))]
   "TARGET_SH1"
-  "clrt")
+  "clrt"
+  [(set_attr "type" "mt_group")])
 
 (define_insn "sett"
   [(set (reg:SI T_REG) (const_int 1))]
   "TARGET_SH1"
-  "sett")
+  "sett"
+  [(set_attr "type" "mt_group")])
 
-
 ;; Use the combine pass to transform sequences such as
 ;;	mov	r5,r0
 ;;	add	#1,r0