diff mbox

[SH] PR 51244 - Improve conditional branches

Message ID 1330970330.2929.261.camel@yam-132-YW-E178-FTW
State New
Headers show

Commit Message

Oleg Endo March 5, 2012, 5:58 p.m. UTC
Hello,

The attached patch is the same as the last one proposed in the PR.
Tested against rev 184877 with

make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a-single/-mb,
-m4-single/-ml,-m4-single/-mb,
-m4a-single/-ml,-m4a-single/-mb}"

and no new failures.
OK?

Cheers,
Oleg

ChangeLog:

	PR target/51244
	* config/sh/sh.c (sh_expand_t_scc): Remove SH2A special case
	and use unified expansion logic.
	* config/sh/sh.md (xorsi3_movrt): Rename to movrt.  Move
	closer to the existing movt insn.
	(negc): Rename insn to *negc.  Add new expander.
	(movnegt): Use xor pattern for T bit negation.  Reserve helper
	constant for negc pattern.
	(*movnegt): New insn and splitter.

testsuite/ChangeLog:

	PR target/51244
	* gcc.target/sh/pr51244-1.c: New.
	* gcc.target/sh/pr51244-2.c: New.
	* gcc.target/sh/pr51244-3.c: New.

Comments

Kaz Kojima March 5, 2012, 10:59 p.m. UTC | #1
Oleg Endo <oleg.endo@t-online.de> wrote:
> The attached patch is the same as the last one proposed in the PR.
> Tested against rev 184877 with
> 
> make -k check RUNTESTFLAGS="--target_board=sh-sim
> \{-m2/-ml,-m2/-mb,-m2a-single/-mb,
> -m4-single/-ml,-m4-single/-mb,
> -m4a-single/-ml,-m4a-single/-mb}"
> 
> and no new failures.
> OK?

OK.

Regards,
	kaz
Kaz Kojima March 5, 2012, 11:13 p.m. UTC | #2
> Oleg Endo <oleg.endo@t-online.de> wrote:
>> The attached patch is the same as the last one proposed in the PR.
>> Tested against rev 184877 with
>> 
>> make -k check RUNTESTFLAGS="--target_board=sh-sim
>> \{-m2/-ml,-m2/-mb,-m2a-single/-mb,
>> -m4-single/-ml,-m4-single/-mb,
>> -m4a-single/-ml,-m4a-single/-mb}"
>> 
>> and no new failures.
>> OK?
> 
> OK.

I've tested your latest patch on sh4-unknown-linux-gnu with trunk
revision 184872.  It looks that some new failures are poping up:

New tests that FAIL:

22_locale/ctype/is/char/3.cc execution test
27_io/basic_filebuf/underflow/wchar_t/9178.cc execution test
gfortran.dg/widechar_intrinsics_6.f90  -Os  execution test

Pehaps failures might be ones you've suggested in #10 in
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51244

Could you double check?

Regards,
	kaz
diff mbox

Patch

Index: gcc/testsuite/gcc.target/sh/pr51244-1.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr51244-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-1.c	(revision 0)
@@ -0,0 +1,32 @@ 
+/* Check that inverted conditional branch logic does not generate
+   unnecessary explicit T bit extractions, inversions and 
+   test instructions.  */
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O1 -mbranch-cost=2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } } */
+/* { dg-final { scan-assembler-not "tst|negc|extu" } } */
+
+int
+testfunc_00 (int a, int b, int c, int d)
+{
+  return (a != b || a != d) ? b : c;
+}
+
+int
+testfunc_01 (int a, char* p, int b, int c)
+{
+  return (a == b && a == c) ? b : c;
+}
+
+int
+testfunc_02 (int a, char* p, int b, int c)
+{
+  return (a == b && a == c) ? b : c;
+}
+
+int
+testfunc_03 (int a, char* p, int b, int c)
+{
+  return (a != b && a != c) ? b : c;
+}
+
Index: gcc/testsuite/gcc.target/sh/pr51244-2.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr51244-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-2.c	(revision 0)
@@ -0,0 +1,18 @@ 
+/* Check that when taking the complement of the T bit using the negc
+   instruction pattern, the constant -1 is loaded only once.
+   On SH2A this test is skipped because the movrt instruction is used
+   to get the complement of the T bit.  */
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O1 -mbranch-cost=2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*" "-m2a*" } { "" } } */
+/* { dg-final { scan-assembler-times "mov\t#-1" 1 } } */
+
+void
+testfunc_00 (int* a, int* b, int c, int d)
+{
+  b[0] = a[0] != c;
+  b[1] = a[1] != d;
+  b[2] = a[2] != c;
+  b[3] = a[3] != d;
+}
+
Index: gcc/testsuite/gcc.target/sh/pr51244-3.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr51244-3.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr51244-3.c	(revision 0)
@@ -0,0 +1,16 @@ 
+/* Check that when taking the complement of the T bit on SH2A, 
+   the movrt instruction is being generated.  */
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O1 -mbranch-cost=2" } */
+/* { dg-skip-if "" { "sh*-*-*" } { "*" } { "-m2a*" } } */
+/* { dg-final { scan-assembler-times "movrt" 4 } } */
+
+void
+testfunc_00 (int* a, int* b, int c, int d)
+{
+  b[0] = a[0] != c;
+  b[1] = a[1] != d;
+  b[2] = a[2] != c;
+  b[3] = a[3] != d;
+}
+
Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c	(revision 184877)
+++ gcc/config/sh/sh.c	(working copy)
@@ -11883,15 +11883,8 @@ 
   val = INTVAL (op1);
   if ((code == EQ && val == 1) || (code == NE && val == 0))
     emit_insn (gen_movt (result));
-  else if (TARGET_SH2A && ((code == EQ && val == 0)
-			    || (code == NE && val == 1)))
-    emit_insn (gen_xorsi3_movrt (result));
   else if ((code == EQ && val == 0) || (code == NE && val == 1))
-    {
-      emit_clobber (result);
-      emit_insn (gen_subc (result, result, result));
-      emit_insn (gen_addsi3 (result, result, const1_rtx));
-    }
+   emit_insn (gen_movnegt (result));
   else if (code == EQ || code == NE)
     emit_insn (gen_move_insn (result, GEN_INT (code == NE)));
   else
Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md	(revision 184877)
+++ gcc/config/sh/sh.md	(working copy)
@@ -3354,15 +3354,6 @@ 
 	xori	%1, %2, %0"
   [(set_attr "type" "arith_media")])
 
-;; Store the complements of the T bit in a register.
-(define_insn "xorsi3_movrt"
-  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
-	(xor:SI (reg:SI T_REG)
-		(const_int 1)))]
-  "TARGET_SH2A"
-  "movrt\\t%0"
-  [(set_attr "type" "arith")])
-
 (define_insn "xordi3"
   [(set (match_operand:DI 0 "arith_reg_dest" "=r,r")
 	(xor:DI (match_operand:DI 1 "arith_reg_operand" "%r,r")
@@ -4387,7 +4378,17 @@ 
 ;; Unary arithmetic
 ;; -------------------------------------------------------------------------
 
-(define_insn "negc"
+(define_expand "negc"
+  [(parallel [(set (match_operand:SI 0 "arith_reg_dest" "")
+	(neg:SI (plus:SI (reg:SI T_REG)
+			 (match_operand:SI 1 "arith_reg_operand" ""))))
+   (set (reg:SI T_REG)
+	(ne:SI (ior:SI (reg:SI T_REG) (match_dup 1))
+	       (const_int 0)))])]
+  ""
+  "")
+
+(define_insn "*negc"
   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
 	(neg:SI (plus:SI (reg:SI T_REG)
 			 (match_operand:SI 1 "arith_reg_operand" "r"))))
@@ -9528,6 +9529,13 @@ 
   "movt	%0"
   [(set_attr "type" "arith")])
 
+(define_insn "movrt"
+  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+	(xor:SI (reg:SI T_REG) (const_int 1)))]
+  "TARGET_SH2A"
+  "movrt	%0"
+  [(set_attr "type" "arith")])
+
 (define_expand "cstore4_media"
   [(set (match_operand:SI 0 "register_operand" "=r")
 	(match_operator:SI 1 "sh_float_comparison_operator"
@@ -9654,40 +9662,55 @@ 
    DONE;
 ")
 
-;; sne moves the complement of the T reg to DEST like this:
-;;      cmp/eq ...
-;;      mov    #-1,temp
-;;      negc   temp,dest
-;;   This is better than xoring compare result with 1 because it does
-;;   not require r0 and further, the -1 may be CSE-ed or lifted out of a
-;;   loop.
+;; Move the complement of the T reg to a reg.
+;; On SH2A the movrt insn can be used.
+;; On anything else than SH2A this has to be done with multiple instructions.
+;; One obvious way would be:
+;;	cmp/eq	...
+;;	movt	r0
+;;	xor	#1,r0
+;;
+;; However, this puts pressure on r0 in most cases and thus the following is
+;; more appealing:
+;;	cmp/eq	...
+;;	mov	#-1,temp
+;;	negc	temp,dest
+;;
+;; If the constant -1 can be CSE-ed or lifted out of a loop it effectively
+;; becomes a one instruction operation.  Moreover, care must be taken that
+;; the insn can still be combined with inverted compare and branch code
+;; around it.
+;; The expander will reserve the constant -1, the insn makes the whole thing
+;; combinable, the splitter finally emits the insn if it was not combined 
+;; away.
+;; Notice that when using the negc variant the T bit also gets inverted.
 
 (define_expand "movnegt"
   [(set (match_dup 1) (const_int -1))
-   (parallel [(set (match_operand:SI 0 "" "")
-		   (neg:SI (plus:SI (reg:SI T_REG)
-				    (match_dup 1))))
-	      (set (reg:SI T_REG)
-		   (ne:SI (ior:SI (reg:SI T_REG) (match_dup 1))
-			  (const_int 0)))])]
+   (parallel [(set (match_operand:SI 0 "arith_reg_dest" "")
+		   (xor:SI (reg:SI T_REG) (const_int 1)))
+   (use (match_dup 1))])]
   ""
-  "
 {
   operands[1] = gen_reg_rtx (SImode);
-}")
+})
 
-;; Recognize mov #-1/negc/neg sequence, and change it to movt/add #-1.
-;; This prevents a regression that occurred when we switched from xor to
-;; mov/neg for sne.
-
-(define_split
-  [(set (match_operand:SI 0 "arith_reg_dest" "")
-	(plus:SI (reg:SI T_REG)
-		 (const_int -1)))]
+(define_insn_and_split "*movnegt"
+  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+	(xor:SI (reg:SI T_REG) (const_int 1)))
+   (use (match_operand:SI 1 "arith_reg_operand" "r"))]
   "TARGET_SH1"
-  [(set (match_dup 0) (eq:SI (reg:SI T_REG) (const_int 1)))
-   (set (match_dup 0) (plus:SI (match_dup 0) (const_int -1)))]
-  "")
+  "#"
+  "&& 1"
+  [(const_int 0)]
+{
+  if (TARGET_SH2A)
+    emit_insn (gen_movrt (operands[0]));
+  else
+    emit_insn (gen_negc (operands[0], operands[1]));
+  DONE;
+}
+  [(set_attr "type" "arith")])
 
 (define_expand "cstoresf4"
   [(set (match_operand:SI 0 "register_operand" "=r")