This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] PR target/30315 i386 missed optimization detecting overflows


On Tue, Aug 07, 2007 at 05:15:56PM +0200, Uros Bizjak wrote:

> You should simply name a vector form of the pattern, like 
> ("*sub<mode>3_xx_overflow1") in the ChangeLog, where quotes could be 
> added according to your personal preference.
> 
> Regarding your patch - IMO the form of the pattern where you only 
> clobber scratch register is missing. You could compare i.e. existing 
> *addsi_2 and *addsi_3 patterns, where your patch currently implements 
> only *addsi_2-like form. This would optimize following example:

   Added. I also added zext versions, but the testcase doesn't trigger them,
so there's another missed optimization there.

> BTW: You introduced excellent approach to macroize these patterns. Do 
> you perhaps plan to macroize existing patterns using these macros as 
> well? ;-)

   Well, for a start, I've chosen the macros such that they can be used for
other patterns as well. I've macroized a little further this time.

   I bootstrapped and tested this revised patch on x86_64-unknown-linux-gnu
with no regressions. Ok for trunk?

2007-08-10  Rask Ingemann Lambertsen  <rask@sygehus.dk>

	PR target/30315
	* config/i386/i386.md (SWI)(addsub): New.
	(*<optab><mode>3_cc_overflow1): New.
	(*<optab><mode>3_cc_overflow2): New.
	(*<optab><mode>3_cconly_overflow1): New.
	(*<optab><mode>3_cconly_overflow2): New.
	(*<optab>si3_zext_cc_overflow1): New.
	(*<optab>si3_zext_cc_overflow2): New.
	* config/i386/predicates.md (fcmov_comparison_operator): Accept
	CCCmode for LTU, GTU, LEU and GEU.
	(ix86_comparison_operator): Likewise.
	(ix86_carry_flag_operator): Carry flag is set if LTU or GTU in CCCmode.
	* gcc/config/i386/i386.c (put_condition_code): Support CCCmode.
	(ix86_cc_mode): Use CCCmode when testing for overflow of PLUS
	or MINUS expressions.

testsuite/

2007-08-10  Rask Ingemann Lambertsen  <rask@sygehus.dk>

	PR target/30315
	* gcc.target/i386/pr30315.c: New.

Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	(revision 127179)
+++ gcc/config/i386/i386.md	(working copy)
@@ -488,6 +488,36 @@
   [(set_attr "length" "128")
    (set_attr "type" "multi")])
 
+(define_code_macro addsub [plus minus])
+
+;; Base name to use for define_insn and such.
+(define_code_attr optab [(plus "add") (minus "sub")])
+
+;; Mark commutative operators as such in constraints.
+(define_code_attr pct [(plus "%") (minus "")])
+
+;; Instruction mnemonic for output templates.
+(define_code_attr mnemonic [(plus "add") (minus "sub")])
+
+;; All single word integer modes.
+(define_mode_macro SWI [QI HI SI (DI "TARGET_64BIT")])
+
+;; Instruction suffix for integer modes.
+(define_mode_attr imodesuffix [(QI "b") (HI "w") (SI "l") (DI "q")])
+
+;; Register class for integer modes.
+(define_mode_attr r [(QI "q") (HI "r") (SI "r") (DI "r")])
+
+;; Immediate operand constraint for integer modes.
+(define_mode_attr i [(QI "i") (HI "i") (SI "i") (DI "e")])
+
+;; General operand predicate for integer modes.
+(define_mode_attr general_operand
+	[(QI "general_operand")
+	 (HI "general_operand")
+	 (SI "general_operand")
+	 (DI "x86_64_general_operand")])
+
 ;; All x87 floating point modes
 (define_mode_macro X87MODEF [SF DF XF])
 
@@ -4855,6 +4885,82 @@
   [(set_attr "type" "alu")
    (set_attr "mode" "DI")])
 
+(define_insn "*<optab><mode>3_cc_overflow1"
+  [(set (reg:CCC FLAGS_REG)
+	(compare:CCC
+	    (addsub:SWI (match_operand:SWI 1 "nonimmediate_operand" "<pct>0,0")
+			(match_operand:SWI 2 "<general_operand>" "<r><i>,<r>m"))
+	    (match_dup 1)))
+   (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m,<r>")
+	(addsub:SWI (match_dup 1) (match_dup 2)))]
+  "ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "<mnemonic>{<imodesuffix>}\t{%2, %0|%0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "<MODE>")])
+
+(define_insn "*<optab><mode>3_cc_overflow2"
+  [(set (reg:CCC FLAGS_REG)
+	(compare:CCC
+	    (addsub:SWI (match_operand:SWI 1 "nonimmediate_operand" "<pct>0,0")
+			(match_operand:SWI 2 "<general_operand>" "<r><i>,<r>m"))
+	    (match_dup 2)))
+   (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m,<r>")
+	(addsub:SWI (match_dup 1) (match_dup 2)))]
+  "ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "<mnemonic>{<imodesuffix>}\t{%2, %0|%0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "<MODE>")])
+
+(define_insn "*<optab><mode>3_cconly_overflow1"
+  [(set (reg:CCC FLAGS_REG)
+	(compare:CCC
+	    (addsub:SWI (match_operand:SWI 1 "nonimmediate_operand" "<pct>0")
+			(match_operand:SWI 2 "<general_operand>" "<r><i>m"))
+	    (match_dup 1)))
+   (clobber (match_scratch:SWI 0 "=<r>"))]
+  "ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "<mnemonic>{<imodesuffix>}\t{%2, %0|%0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "<MODE>")])
+
+(define_insn "*<optab><mode>3_cconly_overflow2"
+  [(set (reg:CCC FLAGS_REG)
+	(compare:CCC
+	    (addsub:SWI (match_operand:SWI 1 "nonimmediate_operand" "<pct>0")
+			(match_operand:SWI 2 "<general_operand>" "<r><i>m"))
+	    (match_dup 2)))
+   (clobber (match_scratch:SWI 0 "=<r>"))]
+  "ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "<mnemonic>{<imodesuffix>}\t{%2, %0|%0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "<MODE>")])
+
+(define_insn "*<optab>si3_zext_cc_overflow1"
+  [(set (reg:CCC FLAGS_REG)
+	(compare:CCC
+	     (addsub:SI (match_operand:SI 1 "nonimmediate_operand" "<pct>0")
+			(match_operand:SI 2 "general_operand" "g"))
+	     (match_dup 1)))
+   (set (match_operand:DI 0 "register_operand" "=r")
+	(zero_extend:DI (addsub:SI (match_dup 1) (match_dup 2))))]
+  "TARGET_64BIT && ix86_binary_operator_ok (<CODE>, SImode, operands)"
+  "<mnemonic>{l}\t{%2, %k0|%k0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "SI")])
+
+(define_insn "*<optab>si3_zext_cc_overflow2"
+  [(set (reg:CCC FLAGS_REG)
+	(compare:CCC
+	     (addsub:SI (match_operand:SI 1 "nonimmediate_operand" "<pct>0")
+			(match_operand:SI 2 "general_operand" "g"))
+	     (match_dup 2)))
+   (set (match_operand:DI 0 "register_operand" "=r")
+	(zero_extend:DI (addsub:SI (match_dup 1) (match_dup 2))))]
+  "TARGET_64BIT && ix86_binary_operator_ok (<CODE>, SImode, operands)"
+  "<mnemonic>{l}\t{%2, %k0|%k0, %2}"
+  [(set_attr "type" "alu")
+   (set_attr "mode" "SI")])
+
 (define_insn "addqi3_carry"
   [(set (match_operand:QI 0 "nonimmediate_operand" "=qm,q")
 	  (plus:QI (plus:QI (match_operand:QI 3 "ix86_carry_flag_operator" "")
Index: gcc/config/i386/predicates.md
===================================================================
--- gcc/config/i386/predicates.md	(revision 127179)
+++ gcc/config/i386/predicates.md	(working copy)
@@ -879,7 +879,8 @@
   switch (code)
     {
     case LTU: case GTU: case LEU: case GEU:
-      if (inmode == CCmode || inmode == CCFPmode || inmode == CCFPUmode)
+      if (inmode == CCmode || inmode == CCFPmode || inmode == CCFPUmode
+	  || inmode == CCCmode)
 	return 1;
       return 0;
     case ORDERED: case UNORDERED:
@@ -924,7 +925,11 @@
 	  || inmode == CCGOCmode || inmode == CCNOmode)
 	return 1;
       return 0;
-    case LTU: case GTU: case LEU: case ORDERED: case UNORDERED: case GEU:
+    case LTU: case GTU: case LEU: case GEU:
+      if (inmode == CCmode || inmode == CCCmode)
+	return 1;
+      return 0;
+    case ORDERED: case UNORDERED:
       if (inmode == CCmode)
 	return 1;
       return 0;
@@ -939,7 +944,7 @@
 
 ;; Return 1 if OP is a valid comparison operator testing carry flag to be set.
 (define_predicate "ix86_carry_flag_operator"
-  (match_code "ltu,lt,unlt,gt,ungt,le,unle,ge,unge,ltgt,uneq")
+  (match_code "ltu,lt,unlt,gtu,gt,ungt,le,unle,ge,unge,ltgt,uneq")
 {
   enum machine_mode inmode = GET_MODE (XEXP (op, 0));
   enum rtx_code code = GET_CODE (op);
@@ -957,6 +962,8 @@
 	return 0;
       code = ix86_fp_compare_code_to_integer (code);
     }
+  else if (inmode == CCCmode)
+   return code == LTU || code == GTU;
   else if (inmode != CCmode)
     return 0;
 
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 127179)
+++ gcc/config/i386/i386.c	(working copy)
@@ -8254,8 +8254,12 @@ put_condition_code (enum rtx_code code, 
     case GTU:
       /* ??? Use "nbe" instead of "a" for fcmov lossage on some assemblers.
 	 Those same assemblers have the same but opposite lossage on cmov.  */
-      gcc_assert (mode == CCmode);
-      suffix = fp ? "nbe" : "a";
+      if (mode == CCmode)
+	suffix = fp ? "nbe" : "a";
+      else if (mode == CCCmode)
+	suffix = "b";
+      else
+	gcc_unreachable ();
       break;
     case LT:
       switch (mode)
@@ -8275,7 +8279,7 @@ put_condition_code (enum rtx_code code, 
 	}
       break;
     case LTU:
-      gcc_assert (mode == CCmode);
+      gcc_assert (mode == CCmode || mode == CCCmode);
       suffix = "b";
       break;
     case GE:
@@ -8297,7 +8301,7 @@ put_condition_code (enum rtx_code code, 
       break;
     case GEU:
       /* ??? As above.  */
-      gcc_assert (mode == CCmode);
+      gcc_assert (mode == CCmode || mode == CCCmode);
       suffix = fp ? "nb" : "ae";
       break;
     case LE:
@@ -8305,8 +8309,13 @@ put_condition_code (enum rtx_code code, 
       suffix = "le";
       break;
     case LEU:
-      gcc_assert (mode == CCmode);
-      suffix = "be";
+      /* ??? As above.  */
+      if (mode == CCmode)
+	suffix = "be";
+      else if (mode == CCCmode)
+	suffix = fp ? "nb" : "ae";
+      else
+	gcc_unreachable ();
       break;
     case UNORDERED:
       suffix = fp ? "u" : "p";
@@ -11033,10 +11042,23 @@ ix86_cc_mode (enum rtx_code code, rtx op
       return CCZmode;
       /* Codes needing carry flag.  */
     case GEU:			/* CF=0 */
-    case GTU:			/* CF=0 & ZF=0 */
     case LTU:			/* CF=1 */
+      /* Detect overflow checks.  They need just the carry flag.  */
+      if (GET_CODE (op0) == PLUS
+	  && (rtx_equal_p (op1, XEXP (op0, 0))
+	      || rtx_equal_p (op1, XEXP (op0, 1))))
+	return CCCmode;
+      else
+	return CCmode;
+    case GTU:			/* CF=0 & ZF=0 */
     case LEU:			/* CF=1 | ZF=1 */
-      return CCmode;
+      /* Detect overflow checks.  They need just the carry flag.  */
+      if (GET_CODE (op0) == MINUS
+	  && (rtx_equal_p (op1, XEXP (op0, 0))
+	      || rtx_equal_p (op1, XEXP (op0, 1))))
+	return CCCmode;
+      else
+	return CCmode;
       /* Codes possibly doable only with sign flag when
          comparing against zero.  */
     case GE:			/* SF=OF   or   SF=0 */
Index: gcc/testsuite/gcc.target/i386/pr30315.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr30315.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr30315.c	(revision 0)
@@ -0,0 +1,101 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not "cmp" } } */
+
+extern void abort (void);
+int c;
+
+#define PLUSCC1(T, t, C)	\
+T pluscc##t##C (T a, T b)	\
+{	\
+  T sum = a + b;	\
+  if (sum < C)		\
+    abort ();		\
+  return sum;		\
+}
+#define PLUSCC(T, t) PLUSCC1(T, t, a) PLUSCC1(T, t, b)
+
+#define INCCC1(T, t, C)	\
+T inccc##t##C (T a, T b)	\
+{	\
+  T sum = a + b;	\
+  if (sum < C)		\
+    c ++;		\
+  return sum;		\
+}
+#define INCCC(T, t) INCCC1(T, t, a) INCCC1(T, t, b)
+
+#define PLUSCCONLY1(T, t, C)	\
+void pluscconly##t##C (T a, T b)	\
+{	\
+  T sum = a + b;	\
+  if (sum < C)		\
+    abort ();		\
+}
+#define PLUSCCONLY(T, t) PLUSCCONLY1(T, t, a) PLUSCCONLY1(T, t, b)
+
+#define MINUSCC1(T, t, C)	\
+T minuscc##t##C (T a, T b)	\
+{	\
+  T difference = a - b;	\
+  if (difference > C)	\
+    abort ();		\
+  return difference;	\
+}
+#define MINUSCC(T, t) MINUSCC1(T, t, a) MINUSCC1(T, t, b)
+
+#define DECCC1(T, t, C)	\
+T deccc##t##C (T a, T b)	\
+{	\
+  T difference = a - b;	\
+  if (difference > C)	\
+    c --;		\
+  return difference;	\
+}
+#define DECCC(T, t) DECCC1(T, t, a) DECCC1(T, t, b)
+
+#define MINUSCCONLY1(T, t, C)	\
+void minuscconly##t##C (T a, T b)	\
+{	\
+  T difference = a - b;	\
+  if (difference > C)	\
+    abort ();		\
+}
+#define MINUSCCONLY(T, t) MINUSCCONLY1(T, t, a) MINUSCCONLY1(T, t, b)
+
+#define TEST(T, t)	\
+  PLUSCC(T, t)		\
+  PLUSCCONLY(T, t)	\
+  INCCC(T, t)		\
+  MINUSCC(T, t)		\
+  MINUSCCONLY(T, t)	\
+  DECCC(T, t)
+
+TEST (unsigned long,  l)
+TEST (unsigned int,   i)
+TEST (unsigned short, s) 
+TEST (unsigned char,  c)
+
+#define PLUSCCZEXT(C)	\
+unsigned long pluscczext##C (unsigned int a, unsigned int b)	\
+{	\
+  unsigned int sum = a + b;	\
+  if (sum < C)		\
+    abort ();		\
+  return sum;		\
+}
+
+PLUSCCZEXT(a)
+PLUSCCZEXT(b)
+
+#define MINUSCCZEXT(C)	\
+unsigned long minuscczext##C (unsigned int a, unsigned int b)	\
+{	\
+  unsigned int difference = a - b;	\
+  if (difference > C)		\
+    abort ();		\
+  return difference;		\
+}
+
+MINUSCCZEXT(a)
+MINUSCCZEXT(b)

-- 
Rask Ingemann Lambertsen


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]