From patchwork Wed Nov 7 12:55:10 2012
ContentType: text/plain; charset="utf8"
MIMEVersion: 1.0
ContentTransferEncoding: 7bit
Subject: Fix fold reassociation (PR c++/55137)
From: Jakub Jelinek
XPatchworkId: 197661
MessageId: <20121107125510.GB1881@tucnak.redhat.com>
To: gccpatches@gcc.gnu.org
Cc: Eric Botcazou , Richard Henderson
Date: Wed, 7 Nov 2012 13:55:10 +0100
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 loop15.c:
+FAIL: gcc.dg/treessa/loop15.c scantreedumptimes optimized "\\\\+" 0
+FAIL: gcc.dg/treessa/loop15.c scantreedumptimes 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 loop15.c testcase as well as the new testcase.
CCing Eric as the author of the last changes in that area.
Bootstrapped/regtested on x86_64linux and i686linux, ok for trunk?
20121107 Jakub Jelinek
PR c++/55137
* foldconst.c (fold_binary_loc) : 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.ctorture/execute/pr55137.c: New test.
Jakub
 gcc/foldconst.c.jj 20121107 09:16:41.929494183 +0100
+++ gcc/foldconst.c 20121107 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 bitpattern
@@ 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 20121107 09:31:45.027160654 +0100
+++ gcc/testsuite/g++.dg/opt/pr55137.C 20121107 09:31:45.028160649 +0100
@@ 0,0 +1,4 @@
+// PR c++/55137
+// { dgdo compile }
+
+enum E { F = 1 + (int) (sizeof (int)  1) };
 gcc/testsuite/gcc.ctorture/execute/pr55137.c.jj 20121107 09:44:11.226768063 +0100
+++ gcc/testsuite/gcc.ctorture/execute/pr55137.c 20121107 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;
+}