This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix fold reassociation (PR c++/55137)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Eric Botcazou <ebotcazou at adacore dot com>, Richard Henderson <rth at redhat dot com>
- Date: Wed, 7 Nov 2012 13:55:10 +0100
- Subject: [PATCH] Fix fold reassociation (PR c++/55137)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
The first (C++) testcase is rejected since my SIZEOF_EXPR folding deferral
changes, the problem is that
-1 + (int) (sizeof (int) - 1)
is first changed into
-1 + (int) ((unsigned) sizeof (int) + UINT_MAX)
and then fold_binary_loc reassociates it in int type into
(int) sizeof (int) + [-2(overflow)], thus introducing overflow where
there hasn't been originally. maybe_const_value then refuses to fold it
into constant due to that. I've fixed that by the moving the TYPE_OVERFLOW
check from before the operation to a check whether the whole reassociation
doesn't introduce overflows (while previously it was testing solely
for overflow introduced on fold_converted lit0/lit1 being combined
together, not e.g. when overflow was introduced by fold_converting lit0
resp. lit1, or for minus_lit{0,1} constants).
Unfortunately that patch lead to regression on loop-15.c:
+FAIL: gcc.dg/tree-ssa/loop-15.c scan-tree-dump-times optimized "\\\\+" 0
+FAIL: gcc.dg/tree-ssa/loop-15.c scan-tree-dump-times optimized "n_. \\\\* n_." 1
as we were no longer reassociating an expression used in number of
iterations calculation.
Looking at that lead me to the second testcase below, which I hope is
valid C, the overflow happens there in unsigned type, thus with defined
wrapping, then there is implementation defined? conversion to signed type
(but all our targets are two's complement), and associate_trees:
was reassociating it in signed type, thus introducing signed overflow
where there wasn't before. Fixed by doing the reassociation in the unsigned
type if (at least) one of the operands is of unsigned type. This fixes
the loop-15.c testcase as well as the new testcase.
CCing Eric as the author of the last changes in that area.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2012-11-07 Jakub Jelinek <jakub@redhat.com>
PR c++/55137
* fold-const.c (fold_binary_loc) <associate>: Don't introduce
TREE_OVERFLOW through reassociation. If type doesn't have defined
overflow, but one or both of the operands do, use the wrapping type
for reassociation and only convert to type at the end.
* g++.dg/opt/pr55137.C: New test.
* gcc.c-torture/execute/pr55137.c: New test.
--- gcc/fold-const.c.jj 2012-11-07 09:16:41.929494183 +0100
+++ gcc/fold-const.c 2012-11-07 09:47:12.227710542 +0100
@@ -10337,6 +10337,7 @@ fold_binary_loc (location_t loc,
{
tree var0, con0, lit0, minus_lit0;
tree var1, con1, lit1, minus_lit1;
+ tree atype = type;
bool ok = true;
/* Split both trees into variables, constants, and literals. Then
@@ -10352,11 +10353,25 @@ fold_binary_loc (location_t loc,
if (code == MINUS_EXPR)
code = PLUS_EXPR;
- /* With undefined overflow we can only associate constants with one
- variable, and constants whose association doesn't overflow. */
+ /* With undefined overflow prefer doing association in a type
+ which wraps on overflow, if that is one of the operand types. */
if ((POINTER_TYPE_P (type) && POINTER_TYPE_OVERFLOW_UNDEFINED)
|| (INTEGRAL_TYPE_P (type) && !TYPE_OVERFLOW_WRAPS (type)))
{
+ if (INTEGRAL_TYPE_P (TREE_TYPE (arg0))
+ && TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg0)))
+ atype = TREE_TYPE (arg0);
+ else if (INTEGRAL_TYPE_P (TREE_TYPE (arg1))
+ && TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg1)))
+ atype = TREE_TYPE (arg1);
+ gcc_assert (TYPE_PRECISION (atype) == TYPE_PRECISION (type));
+ }
+
+ /* With undefined overflow we can only associate constants with one
+ variable, and constants whose association doesn't overflow. */
+ if ((POINTER_TYPE_P (atype) && POINTER_TYPE_OVERFLOW_UNDEFINED)
+ || (INTEGRAL_TYPE_P (atype) && !TYPE_OVERFLOW_WRAPS (atype)))
+ {
if (var0 && var1)
{
tree tmp0 = var0;
@@ -10367,14 +10382,14 @@ fold_binary_loc (location_t loc,
if (CONVERT_EXPR_P (tmp0)
&& INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (tmp0, 0)))
&& (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (tmp0, 0)))
- <= TYPE_PRECISION (type)))
+ <= TYPE_PRECISION (atype)))
tmp0 = TREE_OPERAND (tmp0, 0);
if (TREE_CODE (tmp1) == NEGATE_EXPR)
tmp1 = TREE_OPERAND (tmp1, 0);
if (CONVERT_EXPR_P (tmp1)
&& INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (tmp1, 0)))
&& (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (tmp1, 0)))
- <= TYPE_PRECISION (type)))
+ <= TYPE_PRECISION (atype)))
tmp1 = TREE_OPERAND (tmp1, 0);
/* The only case we can still associate with two variables
is if they are the same, modulo negation and bit-pattern
@@ -10382,16 +10397,6 @@ fold_binary_loc (location_t loc,
if (!operand_equal_p (tmp0, tmp1, 0))
ok = false;
}
-
- if (ok && lit0 && lit1)
- {
- tree tmp0 = fold_convert (type, lit0);
- tree tmp1 = fold_convert (type, lit1);
-
- if (!TREE_OVERFLOW (tmp0) && !TREE_OVERFLOW (tmp1)
- && TREE_OVERFLOW (fold_build2 (code, type, tmp0, tmp1)))
- ok = false;
- }
}
/* Only do something if we found more than two objects. Otherwise,
@@ -10402,10 +10407,16 @@ fold_binary_loc (location_t loc,
+ (lit0 != 0) + (lit1 != 0)
+ (minus_lit0 != 0) + (minus_lit1 != 0))))
{
- var0 = associate_trees (loc, var0, var1, code, type);
- con0 = associate_trees (loc, con0, con1, code, type);
- lit0 = associate_trees (loc, lit0, lit1, code, type);
- minus_lit0 = associate_trees (loc, minus_lit0, minus_lit1, code, type);
+ bool any_overflows = false;
+ if (lit0) any_overflows |= TREE_OVERFLOW (lit0);
+ if (lit1) any_overflows |= TREE_OVERFLOW (lit1);
+ if (minus_lit0) any_overflows |= TREE_OVERFLOW (minus_lit0);
+ if (minus_lit1) any_overflows |= TREE_OVERFLOW (minus_lit1);
+ var0 = associate_trees (loc, var0, var1, code, atype);
+ con0 = associate_trees (loc, con0, con1, code, atype);
+ lit0 = associate_trees (loc, lit0, lit1, code, atype);
+ minus_lit0 = associate_trees (loc, minus_lit0, minus_lit1,
+ code, atype);
/* Preserve the MINUS_EXPR if the negative part of the literal is
greater than the positive part. Otherwise, the multiplicative
@@ -10419,38 +10430,45 @@ fold_binary_loc (location_t loc,
&& tree_int_cst_lt (lit0, minus_lit0))
{
minus_lit0 = associate_trees (loc, minus_lit0, lit0,
- MINUS_EXPR, type);
+ MINUS_EXPR, atype);
lit0 = 0;
}
else
{
lit0 = associate_trees (loc, lit0, minus_lit0,
- MINUS_EXPR, type);
+ MINUS_EXPR, atype);
minus_lit0 = 0;
}
}
+
+ /* Don't introduce overflows through reassociation. */
+ if (!any_overflows
+ && ((lit0 && TREE_OVERFLOW (lit0))
+ || (minus_lit0 && TREE_OVERFLOW (minus_lit0))))
+ return NULL_TREE;
+
if (minus_lit0)
{
if (con0 == 0)
return
fold_convert_loc (loc, type,
associate_trees (loc, var0, minus_lit0,
- MINUS_EXPR, type));
+ MINUS_EXPR, atype));
else
{
con0 = associate_trees (loc, con0, minus_lit0,
- MINUS_EXPR, type);
+ MINUS_EXPR, atype);
return
fold_convert_loc (loc, type,
associate_trees (loc, var0, con0,
- PLUS_EXPR, type));
+ PLUS_EXPR, atype));
}
}
- con0 = associate_trees (loc, con0, lit0, code, type);
+ con0 = associate_trees (loc, con0, lit0, code, atype);
return
fold_convert_loc (loc, type, associate_trees (loc, var0, con0,
- code, type));
+ code, atype));
}
}
--- gcc/testsuite/g++.dg/opt/pr55137.C.jj 2012-11-07 09:31:45.027160654 +0100
+++ gcc/testsuite/g++.dg/opt/pr55137.C 2012-11-07 09:31:45.028160649 +0100
@@ -0,0 +1,4 @@
+// PR c++/55137
+// { dg-do compile }
+
+enum E { F = -1 + (int) (sizeof (int) - 1) };
--- gcc/testsuite/gcc.c-torture/execute/pr55137.c.jj 2012-11-07 09:44:11.226768063 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr55137.c 2012-11-07 09:44:24.380691303 +0100
@@ -0,0 +1,30 @@
+/* PR c++/55137 */
+
+extern void abort (void);
+
+int
+foo (unsigned int x)
+{
+ return ((int) (x + 1U) + 1) < (int) x;
+}
+
+int
+bar (unsigned int x)
+{
+ return (int) (x + 1U) + 1;
+}
+
+int
+baz (unsigned int x)
+{
+ return x + 1U;
+}
+
+int
+main ()
+{
+ if (foo (__INT_MAX__) != (bar (__INT_MAX__) < __INT_MAX__)
+ || foo (__INT_MAX__) != ((int) baz (__INT_MAX__) + 1 < __INT_MAX__))
+ abort ();
+ return 0;
+}
Jakub