This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH v2] PR target/30315 i386 missed optimization detecting overflows
- From: Rask Ingemann Lambertsen <rask at sygehus dot dk>
- To: gcc-patches at gcc dot gnu dot org
- Date: Sat, 4 Aug 2007 15:06:58 +0200
- Subject: [PATCH v2] PR target/30315 i386 missed optimization detecting overflows
- References: <20070803193457.GQ25795@sygehus.dk>
Here is a revised patch. Compared with the previous one, I
1) Fixed the ix86_carry_flag_operator predicate to not reject GTU.
2) Added HImode and QImode versions using a mode macro.
3) Changed the testcase to use macros and test "unsigned short" and
"unsigned char" also. Additionally, it now tests that constructs like
if (sum < a)
c ++;
and
if (difference > b)
c --;
are optimized too. This revealed the bug mentioned in 1).
Multiword modes such as uint128_t (or uint64_t with -m32) are still not
optimized. Additional insn patterns and splitters are needed for that.
Bootstrapped and tested on x86_64-unknown-linux-gnu with no regressions.
Ok for trunk?
:ADDPATCH target i386:
2007-08-04 Rask Ingemann Lambertsen <rask@sygehus.dk>
PR target/30315
* config/i386/i386.md (*adddi3_cc_overflow1_rex64): New.
(*adddi3_cc_overflow2_rex64): New.
(*addsi3_cc_overflow1): New.
(*addsi3_cc_overflow2): New.
(*addhi3_cc_overflow1): New.
(*addhi3_cc_overflow2): New.
(*addqi3_cc_overflow1): New.
(*addqi3_cc_overflow2): New.
(*subdi3_cc_overflow1_rex64): New.
(*subdi3_cc_overflow2_rex64): New.
(*subsi3_cc_overflow1): New.
(*subsi3_cc_overflow2): New.
(*subhi3_cc_overflow1): New.
(*subhi3_cc_overflow2): New.
(*subqi3_cc_overflow1): New.
(*subqi3_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 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-04 Rask Ingemann Lambertsen <rask@sygehus.dk>
PR target/30315
* gcc/testsuite/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,15 @@
[(set_attr "length" "128")
(set_attr "type" "multi")])
+;; All integer modes of at most 32 bits.
+(define_mode_macro LE32I [QI HI SI])
+
+;; Instruction suffix for integer modes.
+(define_mode_attr imodesuffix [(QI "b") (HI "w") (SI "l")])
+
+;; Register class for integer modes.
+(define_mode_attr r [(QI "q") (HI "r") (SI "r")])
+
;; All x87 floating point modes
(define_mode_macro X87MODEF [SF DF XF])
@@ -4855,6 +4864,32 @@
[(set_attr "type" "alu")
(set_attr "mode" "DI")])
+(define_insn "*adddi3_cc_overflow1_rex64"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (plus:DI (match_operand:DI 1 "nonimmediate_operand" "%0,0")
+ (match_operand:DI 2 "x86_64_general_operand" "re,rm"))
+ (match_dup 1)))
+ (set (match_operand:DI 0 "nonimmediate_operand" "=rm,r")
+ (plus:DI (match_dup 1) (match_dup 2)))]
+ "TARGET_64BIT && ix86_binary_operator_ok (PLUS, DImode, operands)"
+ "add{q}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "DI")])
+
+(define_insn "*adddi3_cc_overflow2_rex64"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (plus:DI (match_operand:DI 1 "nonimmediate_operand" "%0,0")
+ (match_operand:DI 2 "x86_64_general_operand" "re,rm"))
+ (match_dup 2)))
+ (set (match_operand:DI 0 "nonimmediate_operand" "=rm,r")
+ (plus:DI (match_dup 1) (match_dup 2)))]
+ "TARGET_64BIT && ix86_binary_operator_ok (PLUS, DImode, operands)"
+ "add{q}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "DI")])
+
(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" "")
@@ -4916,6 +4951,32 @@
[(set_attr "type" "alu")
(set_attr "mode" "SI")])
+(define_insn "*add<mode>3_cc_overflow1"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (plus:LE32I (match_operand:LE32I 1 "nonimmediate_operand" "%0,0")
+ (match_operand:LE32I 2 "general_operand" "<r>i,<r>m"))
+ (match_dup 1)))
+ (set (match_operand:LE32I 0 "nonimmediate_operand" "=<r>m,<r>")
+ (plus:LE32I (match_dup 1) (match_dup 2)))]
+ "ix86_binary_operator_ok (PLUS, <MODE>mode, operands)"
+ "add{<imodesuffix>}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "<MODE>")])
+
+(define_insn "*add<mode>3_cc_overflow2"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (plus:LE32I (match_operand:LE32I 1 "nonimmediate_operand" "%0,0")
+ (match_operand:LE32I 2 "general_operand" "<r>i,<r>m"))
+ (match_dup 2)))
+ (set (match_operand:LE32I 0 "nonimmediate_operand" "=<r>m,<r>")
+ (plus:LE32I (match_dup 1) (match_dup 2)))]
+ "ix86_binary_operator_ok (PLUS, <MODE>mode, operands)"
+ "add{<imodesuffix>}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "<MODE>")])
+
(define_insn "addqi3_cc"
[(set (reg:CC FLAGS_REG)
(unspec:CC [(match_operand:QI 1 "nonimmediate_operand" "%0,0")
@@ -6608,6 +6669,32 @@
[(set_attr "type" "alu")
(set_attr "mode" "DI")])
+(define_insn "*subdi3_cc_overflow1_rex64"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (minus:DI (match_operand:DI 1 "nonimmediate_operand" "0,0")
+ (match_operand:DI 2 "x86_64_general_operand" "re,rm"))
+ (match_dup 1)))
+ (set (match_operand:DI 0 "nonimmediate_operand" "=rm,r")
+ (minus:DI (match_dup 1) (match_dup 2)))]
+ "TARGET_64BIT && ix86_binary_operator_ok (MINUS, DImode, operands)"
+ "sub{q}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "DI")])
+
+(define_insn "*subdi3_cc_overflow2_rex64"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (minus:DI (match_operand:DI 1 "nonimmediate_operand" "0,0")
+ (match_operand:DI 2 "x86_64_general_operand" "re,rm"))
+ (match_dup 2)))
+ (set (match_operand:DI 0 "nonimmediate_operand" "=rm,r")
+ (minus:DI (match_dup 1) (match_dup 2)))]
+ "TARGET_64BIT && ix86_binary_operator_ok (MINUS, DImode, operands)"
+ "sub{q}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "DI")])
+
(define_insn "subqi3_carry"
[(set (match_operand:QI 0 "nonimmediate_operand" "=qm,q")
(minus:QI (match_operand:QI 1 "nonimmediate_operand" "0,0")
@@ -6700,6 +6787,32 @@
[(set_attr "type" "alu")
(set_attr "mode" "SI")])
+(define_insn "*sub<mode>3_cc_overflow1"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (minus:LE32I (match_operand:LE32I 1 "nonimmediate_operand" "0,0")
+ (match_operand:LE32I 2 "general_operand" "<r>i,<r>m"))
+ (match_dup 1)))
+ (set (match_operand:LE32I 0 "nonimmediate_operand" "=<r>m,<r>")
+ (minus:LE32I (match_dup 1) (match_dup 2)))]
+ "ix86_binary_operator_ok (MINUS, <MODE>mode, operands)"
+ "sub{<imodesuffix>}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "<MODE>")])
+
+(define_insn "*sub<mode>3_cc_overflow2"
+ [(set (reg:CCC FLAGS_REG)
+ (compare:CCC
+ (minus:LE32I (match_operand:LE32I 1 "nonimmediate_operand" "0,0")
+ (match_operand:LE32I 2 "general_operand" "<r>i,<r>m"))
+ (match_dup 2)))
+ (set (match_operand:LE32I 0 "nonimmediate_operand" "=<r>m,<r>")
+ (minus:LE32I (match_dup 1) (match_dup 2)))]
+ "ix86_binary_operator_ok (MINUS, <MODE>mode, operands)"
+ "sub{<imodesuffix>}\t{%2, %0|%0, %2}"
+ [(set_attr "type" "alu")
+ (set_attr "mode" "<MODE>")])
+
(define_insn "*subsi_2_zext"
[(set (reg FLAGS_REG)
(compare
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,53 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not "cmp" } } */
+
+extern void abort (void);
+long int c;
+
+#define FOO1(T, t, C) \
+T foo##t##C (T a, T b) \
+{ \
+ T sum = a + b; \
+ if (sum < C) \
+ abort (); \
+ return sum; \
+}
+#define FOO(T, t) FOO1(T, t, a) FOO1(T, t, b)
+
+#define QWE1(T, t, C) \
+T qwe##t##C (T a, T b) \
+{ \
+ T sum = a + b; \
+ if (sum < C) \
+ c ++; \
+ return sum; \
+}
+#define QWE(T, t) QWE1(T, t, a) QWE1(T, t, b)
+
+#define BAR1(T, t, C) \
+T bar##t##C (T a, T b) \
+{ \
+ T difference = a - b; \
+ if (difference > C) \
+ abort (); \
+ return difference; \
+}
+#define BAR(T, t) BAR1(T, t, a) BAR1(T, t, b)
+
+#define BAZ1(T, t, C) \
+T baz##t##C (T a, T b) \
+{ \
+ T difference = a - b; \
+ if (difference > C) \
+ c --; \
+ return difference; \
+}
+#define BAZ(T, t) BAZ1(T, t, a) BAZ1(T, t, b)
+
+#define TEST(T, t) FOO(T, t) BAR(T, t) BAZ(T, t) QWE(T, t)
+
+TEST (unsigned long, l)
+TEST (unsigned int, i)
+TEST (unsigned short, s)
+TEST (unsigned char, c)
--
Rask Ingemann Lambertsen