int foo(int i) { if (((unsigned)(i + 1)) * 4 == 0) return 1; return 0; } extern void abort(void); int main() { if (foo(0x3fffffff) == 0) abort (); return 0; } This goes wrong in extract_muldiv and is exposed by folding X * C CMP 0 to X CMP 0 for undefined overflow.
Another one: int foo(int i) { return ((int)((unsigned)(i + 1) * 4)) / 4; } extern void abort(void); int main() { if (foo(0x3fffffff) == 0) abort (); return 0; }
whoops, make the testcase in comment #1 int foo(int i) { return ((int)((unsigned)(i + 1) * 4)) / 4; } extern void abort(void); int main() { if (foo(0x3fffffff) != 0) abort (); return 0; }
Only the first testcase is a regression (AFAIK), the second one also fails with 2.95.
A regression hunt for the first testcase on powerpc-linux identified the following patch: http://gcc.gnu.org/viewcvs?view=rev&rev=120649 r120649 | ian | 2007-01-10 21:07:38 +0000 (Wed, 10 Jan 2007)
I looked at this a little. This test is a little bit funny because the '+' has undefined overflow but the '*' does not. Move the cast to make the '+' have defined overflow, and it works. What happens is that we call fold_sign_changed_comparison on the '==' expression (in the very first test case). This decides that the expression can be simplified by removing the casts to unsigned. Some earlier call has already hoisted the cast to unsigned to apply to the '*' tree as a whole, so stripping this means we are optimizing a signed multiply. Offhand I would say that it is not valid to convert an unsigned operation to a signed operation, because that changes overflow from defined to undefined. Going the other direction should generally be ok -- though, I suppose, sometimes miss an optimization. Removing the special case for EQ_EXPR and NE_EXPR from fold_sign_changed_comparison should fix this bug, but I don't know at what cost. Or, perhaps this test could be changed to look at whether arg0_inner is now a signed binary expression. Some advice from a fold expert would be appreciated.
"This goes wrong in extract_muldiv..." sorry for not pasting my complete analysis (actually the testcase is carefuly crafted from looking at this code ;)). The recursion through conversions case CONVERT_EXPR: case NON_LVALUE_EXPR: case NOP_EXPR: /* If op0 is an expression ... */ if ((COMPARISON_CLASS_P (op0) || UNARY_CLASS_P (op0) || BINARY_CLASS_P (op0) || VL_EXP_CLASS_P (op0) || EXPRESSION_CLASS_P (op0)) /* ... and is unsigned, and its type is smaller than ctype, then we cannot pass through as widening. */ && ((TYPE_UNSIGNED (TREE_TYPE (op0)) && ! (TREE_CODE (TREE_TYPE (op0)) == INTEGER_TYPE && TYPE_IS_SIZETYPE (TREE_TYPE (op0))) && (GET_MODE_SIZE (TYPE_MODE (ctype)) > GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0))))) /* ... or this is a truncation (t is narrower than op0), then we cannot pass through this narrowing. */ || (GET_MODE_SIZE (TYPE_MODE (type)) < GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))) /* ... or signedness changes for division or modulus, then we cannot pass through this conversion. */ || (code != MULT_EXPR && (TYPE_UNSIGNED (ctype) != TYPE_UNSIGNED (TREE_TYPE (op0)))))) break; /* Pass the constant down and see if we can make a simplification. If we can, replace this expression with the inner simplification for possible later conversion to our or some other type. */ if ((t2 = fold_convert (TREE_TYPE (op0), c)) != 0 && TREE_CODE (t2) == INTEGER_CST && !TREE_OVERFLOW (t2) && (0 != (t1 = extract_muldiv (op0, t2, code, code == MULT_EXPR ? ctype : NULL_TREE, strict_overflow_p)))) return t1; break; is wrong as it might convert the transformation from one in unsigned type to one in signed type.
I'll take this.
Subject: Bug 33779 Author: rguenth Date: Wed Oct 31 12:33:05 2007 New Revision: 129796 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129796 Log: 2007-10-31 Richard Guenther <rguenther@suse.de> PR middle-end/33779 * fold-const.c (extract_muldiv_1): Make sure to not introduce new undefined integer overflow. (fold_binary): Avoid useless conversion. * gcc.c-torture/execute/pr33779-1.c: New testcase. * gcc.c-torture/execute/pr33779-2.c: Likewise. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr33779-1.c trunk/gcc/testsuite/gcc.c-torture/execute/pr33779-2.c Modified: trunk/gcc/ChangeLog trunk/gcc/fold-const.c trunk/gcc/testsuite/ChangeLog
Fixed.