From 1dfc50232dcb703454db4f54c538042a32be2138 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Fri, 17 Apr 2020 16:56:12 +0200 Subject: [PATCH] i386: Fix up *testqi_ext_3 define_insn_and_split [PR94567] As the testcase shows, there are unfortunately more problematic cases in *testqi_ext_3 if the mode is not CCZmode, because the sign flag might not behave the same between the insn with zero_extract and what we split it into. The previous fix to the insn condition was because *testdi_1 for mask with upper 32-bits clear and bit 31 set is implemented using SImode test and thus SF is set depending on that bit 31 rather than on always cleared. But we can have other cases. On the zero_extract (which has mode), we can have either the pos + len == precision of mode, or pos + len < precision of mode cases. The former one copies the most significant bit into SF, the latter will have SF always cleared. For the former case, either it is a zero_extract from a larger mode, but then when we perform test in that larger mode, SF will be always clear and thus mismatch from the zero_extract case (so we need to enforce CCZmode), or it will be a zero_extract from same mode with pos 0 and len equal to mode precision, such zero_extracts should have been really simplified into their first operand. For the latter case, when SF is always clear on the define_insn with zero_extract, we need to split into something that doesn't sometimes set SF, i.e. it has to be a test with mask that doesn't have the most significant bit set. In some cases it can be achieved through using test in a wider mode (e.g. in the testcase, there is (zero_extract:SI (reg:HI) (const_int 13) (const_int 3)) which will always set SF to 0, but we split it into (and:HI (reg:HI) (const_int -8)) which will copy the MSB of (reg:HI) into SF, but we can do: (and:SI (subreg:SI (reg:HI) 0) (const_int 0xfff8)) which will keep SF always cleared), but there are various cases where we can't (when already using DImode, or when SImode and we'd turned it into the problematic *testdi_1 implemented using SImode test, or when the val operand is a MEM (we don't want to read from memory more than the user originally wanted), paradoxical subreg of MEM could be problematic too if we through the narrowing end up with a MEM). So, the patch attempts to require CCZmode (and not CCNOmode) if it can't really ensure the SF will have same meaning between the define_insn and what we split it into, and if we decide we allow CCNOmode, it needs to avoid performing narrowing and/or widen if pos + len would indicate we'd have MSB set in the mask. 2020-04-17 Jakub Jelinek Jeff Law PR target/94567 * config/i386/i386.md (*testqi_ext_3): Use CCZmode rather than CCNOmode in ix86_match_ccmode if len is equal to mode precision, or pos + len >= 32, or pos + len is equal to operands[2] precision and operands[2] is not a register operand. During splitting perform SImode AND if operands[0] doesn't have CCZmode and pos + len is equal to mode precision. * gcc.c-torture/execute/pr94567.c: New test. Co-Authored-By: Jeff Law --- gcc/ChangeLog | 11 ++++ gcc/config/i386/i386.md | 63 ++++++++++++++++--- gcc/testsuite/ChangeLog | 6 ++ gcc/testsuite/gcc.c-torture/execute/pr94567.c | 26 ++++++++ 4 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94567.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index d2aa482b2e0..1c010560c19 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,14 @@ +2020-04-17 Jakub Jelinek + Jeff Law + + PR target/94567 + * config/i386/i386.md (*testqi_ext_3): Use CCZmode rather than + CCNOmode in ix86_match_ccmode if len is equal to mode precision, + or pos + len >= 32, or pos + len is equal to operands[2] precision + and operands[2] is not a register operand. During splitting perform + SImode AND if operands[0] doesn't have CCZmode and pos + len is + equal to mode precision. + 2020-04-17 Richard Biener PR other/94629 diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 3051624d89f..b426c21d3dd 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -8730,10 +8730,38 @@ && INTVAL (operands[3]) > 32 && INTVAL (operands[3]) + INTVAL (operands[4]) == 64)) && ix86_match_ccmode (insn, - /* *testdi_1 requires CCZmode if the mask has bit + /* If zero_extract mode precision is the same + as len, the SF of the zero_extract + comparison will be the most significant + extracted bit, but this could be matched + after splitting only for pos 0 len all bits + trivial extractions. Require CCZmode. */ + (GET_MODE_PRECISION (mode) + == INTVAL (operands[3])) + /* Otherwise, require CCZmode if we'd use a mask + with the most significant bit set and can't + widen it to wider mode. *testdi_1 also + requires CCZmode if the mask has bit 31 set and all bits above it clear. */ - GET_MODE (operands[2]) == DImode - && INTVAL (operands[3]) + INTVAL (operands[4]) == 32 + || (INTVAL (operands[3]) + INTVAL (operands[4]) + >= 32) + /* We can't widen also if val is not a REG. */ + || (INTVAL (operands[3]) + INTVAL (operands[4]) + == GET_MODE_PRECISION (GET_MODE (operands[2])) + && !register_operand (operands[2], + GET_MODE (operands[2]))) + /* And we shouldn't widen if + TARGET_PARTIAL_REG_STALL. */ + || (TARGET_PARTIAL_REG_STALL + && (INTVAL (operands[3]) + INTVAL (operands[4]) + >= (paradoxical_subreg_p (operands[2]) + && (GET_MODE_CLASS + (GET_MODE (SUBREG_REG (operands[2]))) + == MODE_INT) + ? GET_MODE_PRECISION + (GET_MODE (SUBREG_REG (operands[2]))) + : GET_MODE_PRECISION + (GET_MODE (operands[2]))))) ? CCZmode : CCNOmode)" "#" "&& 1" @@ -8750,7 +8778,10 @@ /* Narrow paradoxical subregs to prevent partial register stalls. */ if (GET_MODE_BITSIZE (mode) > GET_MODE_BITSIZE (submode) - && GET_MODE_CLASS (submode) == MODE_INT) + && GET_MODE_CLASS (submode) == MODE_INT + && (GET_MODE (operands[0]) == CCZmode + || pos + len < GET_MODE_PRECISION (submode) + || REG_P (SUBREG_REG (val)))) { val = SUBREG_REG (val); mode = submode; @@ -8758,14 +8789,32 @@ } /* Small HImode tests can be converted to QImode. */ - if (register_operand (val, HImode) && pos + len <= 8) + if (pos + len <= 8 + && register_operand (val, HImode)) { - val = gen_lowpart (QImode, val); - mode = QImode; + rtx nval = gen_lowpart (QImode, val); + if (!MEM_P (nval) + || GET_MODE (operands[0]) == CCZmode + || pos + len < 8) + { + val = nval; + mode = QImode; + } } gcc_assert (pos + len <= GET_MODE_PRECISION (mode)); + /* If the mask is going to have the sign bit set in the mode + we want to do the comparison in and user isn't interested just + in the zero flag, then we must widen the target mode. */ + if (pos + len == GET_MODE_PRECISION (mode) + && GET_MODE (operands[0]) != CCZmode) + { + gcc_assert (pos + len < 32 && !MEM_P (val)); + mode = SImode; + val = gen_lowpart (mode, val); + } + wide_int mask = wi::shifted_mask (pos, len, false, GET_MODE_PRECISION (mode)); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index b98c72cdd2a..830ee92357e 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2020-04-17 Jakub Jelinek + Jeff Law + + PR target/94567 + * gcc.c-torture/execute/pr94567.c: New test. + 2020-04-17 Nathan Sidwell PR c++/94608 diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94567.c b/gcc/testsuite/gcc.c-torture/execute/pr94567.c new file mode 100644 index 00000000000..679d73d2ef4 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr94567.c @@ -0,0 +1,26 @@ +/* PR target/94567 */ + +volatile int a = 1, b; +short c, d = 4, f = 2, g; +unsigned short e = 53736; + +int +foo (int i, int j) +{ + return i && j ? 0 : i + j; +} + +int +main () +{ + for (; a; a = 0) + { + unsigned short k = e; + g = k >> 3; + if (foo (g < (f || c), b)) + d = 0; + } + if (d != 4) + __builtin_abort (); + return 0; +} -- 2.30.2