From c124b345e460780e3c3d4a6e58d0e6e03da982a1 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Thu, 23 Jan 2020 16:17:56 +0100 Subject: [PATCH] i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376] The following testcase ICEs, because during try_combine of i3: (insn 18 17 19 2 (parallel [ (set (reg:CCO 17 flags) (eq:CCO (plus:OI (sign_extend:OI (reg:TI 96)) (const_int 1 [0x1])) (sign_extend:OI (plus:TI (reg:TI 96) (const_int 1 [0x1]))))) (set (reg:TI 98) (plus:TI (reg:TI 96) (const_int 1 [0x1]))) ]) "pr93376.c":8:10 223 {*addvti4_doubleword_1} (expr_list:REG_UNUSED (reg:TI 98) (expr_list:REG_DEAD (reg:TI 96) (nil)))) and i2: (insn 17 37 18 2 (set (reg:TI 96) (const_wide_int 0x7fffffffffffffffffffffffffffffff)) "pr93376.c":8:10 65 {*movti_internal} (nil)) the eq in there gets simplified into: (eq:CCO (const_wide_int 0x080000000000000000000000000000000) (const_wide_int 0x80000000000000000000000000000000)) and simplify-rtx.c tries to simplify it by simplifying MINUS of the two operands. Now, i386 defines MAX_BITSIZE_MODE_ANY_INT to 128, because OImode and XImode are used mainly as a placeholder for the vector modes; these new signed overflow patterns are an exception to that, but what they really need is just TImode precision + 1 (maybe 2 worst case) bits at any time. wide-int.h defines WIDE_INT_MAX_ELTS in a way that it contains one more HWI above number of HWIs to cover WIDE_INT_MAX_ELTS, so on i386 that is 3 HWIs, meaning that TImode precision + 1/2 bits is still representable in there. Unfortunately, the way wi::sub_large is implemented, it needs not just those 3 HWIs, but one HWI above the maximum of the lengths of both operands, which means it buffer overflows, overwrites the following precision in wide_int_storage and ICEs later on. The need for 4 HWIs is only temporary, because canonize immediately after it canonicalizes it back to 3 HWIs only. The patch is something suggested by Richard S., avoid using OImode for this and instead use a partial int mode that is smaller. 2020-01-23 Jakub Jelinek PR target/93376 * config/i386/i386-modes.def (POImode): New mode. (MAX_BITSIZE_MODE_ANY_INT): Change from 128 to 160. * config/i386/i386.md (DPWI): New mode attribute. (addv4, subv4): Use instead of . (QWI): Rename to... (QPWI): ... this. Use POI instead of OI for TImode. (*addv4_doubleword, *addv4_doubleword_1, *subv4_doubleword, *subv4_doubleword_1): Use instead of . * gcc.dg/pr93376.c: New test. --- gcc/ChangeLog | 13 +++++++++ gcc/config/i386/i386-modes.def | 13 +++++++-- gcc/config/i386/i386.md | 53 ++++++++++++++++++---------------- gcc/testsuite/ChangeLog | 5 ++++ gcc/testsuite/gcc.dg/pr93376.c | 20 +++++++++++++ 5 files changed, 77 insertions(+), 27 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr93376.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ac8b18f1a84..3d50dabe603 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,16 @@ +2020-01-23 Jakub Jelinek + + PR target/93376 + * config/i386/i386-modes.def (POImode): New mode. + (MAX_BITSIZE_MODE_ANY_INT): Change from 128 to 160. + * config/i386/i386.md (DPWI): New mode attribute. + (addv4, subv4): Use instead of . + (QWI): Rename to... + (QPWI): ... this. Use POI instead of OI for TImode. + (*addv4_doubleword, *addv4_doubleword_1, + *subv4_doubleword, *subv4_doubleword_1): Use + instead of . + 2020-01-23 Richard Sandiford PR target/93341 diff --git a/gcc/config/i386/i386-modes.def b/gcc/config/i386/i386-modes.def index bb578011ad6..4278bb82281 100644 --- a/gcc/config/i386/i386-modes.def +++ b/gcc/config/i386/i386-modes.def @@ -107,10 +107,19 @@ INT_MODE (XI, 64); PARTIAL_INT_MODE (HI, 16, P2QI); PARTIAL_INT_MODE (SI, 32, P2HI); +/* Mode used for signed overflow checking of TImode. As + MAX_BITSIZE_MODE_ANY_INT is only 160, wide-int.h reserves only that + rounded up to multiple of HOST_BITS_PER_WIDE_INT bits in wide_int etc., + so OImode is too large. For the overflow checking we actually need + just 1 or 2 bits beyond TImode precision. Use 160 bits to have + a multiple of 32. */ +PARTIAL_INT_MODE (OI, 160, POI); + /* Keep the OI and XI modes from confusing the compiler into thinking that these modes could actually be used for computation. They are - only holders for vectors during data movement. */ -#define MAX_BITSIZE_MODE_ANY_INT (128) + only holders for vectors during data movement. Include POImode precision + though. */ +#define MAX_BITSIZE_MODE_ANY_INT (160) /* The symbol Pmode stands for one of the above machine modes (usually SImode). The tm.h file specifies which one. It is not a distinct mode. */ diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index b5b53bbcfaa..98d452aecfe 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -6054,15 +6054,18 @@ [(set_attr "type" "alu") (set_attr "mode" "QI")]) +;; Like DWI, but use POImode instead of OImode. +(define_mode_attr DPWI [(QI "HI") (HI "SI") (SI "DI") (DI "TI") (TI "POI")]) + ;; Add with jump on overflow. (define_expand "addv4" [(parallel [(set (reg:CCO FLAGS_REG) (eq:CCO - (plus: - (sign_extend: + (plus: + (sign_extend: (match_operand:SWIDWI 1 "nonimmediate_operand")) (match_dup 4)) - (sign_extend: + (sign_extend: (plus:SWIDWI (match_dup 1) (match_operand:SWIDWI 2 ""))))) @@ -6078,7 +6081,7 @@ if (CONST_SCALAR_INT_P (operands[2])) operands[4] = operands[2]; else - operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); + operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); }) (define_insn "*addv4" @@ -6123,17 +6126,17 @@ (const_string "")))]) ;; Quad word integer modes as mode attribute. -(define_mode_attr QWI [(SI "TI") (DI "OI")]) +(define_mode_attr QPWI [(SI "TI") (DI "POI")]) (define_insn_and_split "*addv4_doubleword" [(set (reg:CCO FLAGS_REG) (eq:CCO - (plus: - (sign_extend: + (plus: + (sign_extend: (match_operand: 1 "nonimmediate_operand" "%0,0")) - (sign_extend: + (sign_extend: (match_operand: 2 "x86_64_hilo_general_operand" "r,o"))) - (sign_extend: + (sign_extend: (plus: (match_dup 1) (match_dup 2))))) (set (match_operand: 0 "nonimmediate_operand" "=ro,r") (plus: (match_dup 1) (match_dup 2)))] @@ -6172,11 +6175,11 @@ (define_insn_and_split "*addv4_doubleword_1" [(set (reg:CCO FLAGS_REG) (eq:CCO - (plus: - (sign_extend: + (plus: + (sign_extend: (match_operand: 1 "nonimmediate_operand" "%0")) - (match_operand: 3 "const_scalar_int_operand" "")) - (sign_extend: + (match_operand: 3 "const_scalar_int_operand" "")) + (sign_extend: (plus: (match_dup 1) (match_operand: 2 "x86_64_hilo_general_operand" ""))))) @@ -6570,11 +6573,11 @@ (define_expand "subv4" [(parallel [(set (reg:CCO FLAGS_REG) (eq:CCO - (minus: - (sign_extend: + (minus: + (sign_extend: (match_operand:SWIDWI 1 "nonimmediate_operand")) (match_dup 4)) - (sign_extend: + (sign_extend: (minus:SWIDWI (match_dup 1) (match_operand:SWIDWI 2 ""))))) @@ -6590,7 +6593,7 @@ if (CONST_SCALAR_INT_P (operands[2])) operands[4] = operands[2]; else - operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); + operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); }) (define_insn "*subv4" @@ -6637,12 +6640,12 @@ (define_insn_and_split "*subv4_doubleword" [(set (reg:CCO FLAGS_REG) (eq:CCO - (minus: - (sign_extend: + (minus: + (sign_extend: (match_operand: 1 "nonimmediate_operand" "0,0")) - (sign_extend: + (sign_extend: (match_operand: 2 "x86_64_hilo_general_operand" "r,o"))) - (sign_extend: + (sign_extend: (minus: (match_dup 1) (match_dup 2))))) (set (match_operand: 0 "nonimmediate_operand" "=ro,r") (minus: (match_dup 1) (match_dup 2)))] @@ -6679,11 +6682,11 @@ (define_insn_and_split "*subv4_doubleword_1" [(set (reg:CCO FLAGS_REG) (eq:CCO - (minus: - (sign_extend: + (minus: + (sign_extend: (match_operand: 1 "nonimmediate_operand" "0")) - (match_operand: 3 "const_scalar_int_operand" "")) - (sign_extend: + (match_operand: 3 "const_scalar_int_operand" "")) + (sign_extend: (minus: (match_dup 1) (match_operand: 2 "x86_64_hilo_general_operand" ""))))) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 7a03ebbaef8..fef595104fe 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-01-23 Jakub Jelinek + + PR target/93376 + * gcc.dg/pr93376.c: New test. + 2020-01-23 Richard Sandiford PR target/93341 diff --git a/gcc/testsuite/gcc.dg/pr93376.c b/gcc/testsuite/gcc.dg/pr93376.c new file mode 100644 index 00000000000..49341ff0a4b --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr93376.c @@ -0,0 +1,20 @@ +/* PR target/93376 */ +/* { dg-do compile { target int128 } } */ +/* { dg-options "-Og -finline-functions-called-once" } */ + +unsigned a, b, c; + +int +bar (int x) +{ + short s = __builtin_sub_overflow (~x, 0, &b); + a = __builtin_ffsll (~x); + return __builtin_add_overflow_p (-(unsigned __int128) a, s, + (unsigned __int128) 0); +} + +void +foo (void) +{ + c = bar (0); +} -- 2.30.2