On 09/09/13 10:56, Kyrylo Tkachov wrote:
> [Resending, since I was away and not pinging it]
>
>
> Hi all,
>
> In PR58088 the constant folder goes into an infinite recursion and runs out of
> stack space because of two conflicting optimisations:
> (X * C1) & C2 plays dirty when nested inside an IOR expression like so: ((X *
> C1) & C2) | C4. One can undo the other leading to an infinite recursion.
>
> Thanks to Marek for finding the IOR case.
>
> This patch fixes that by checking in the IOR case that the change to C2 will
> not conflict with the AND case transformation. Example testcases in the PR on
> bugzilla.
>
> This affects both trunk and 4.8 and regresses and bootstraps cleanly on both.
>
> Bootstrapped on x86_64-linux-gnu and tested arm-none-eabi on qemu.
>
> Ok for trunk and 4.8?
>
> Thanks,
> Kyrill
>
> 2013-09-09 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
>
> PR tree-optimization/58088
> * fold-const.c (mask_with_trailing_zeros): New function.
> (fold_binary_loc): Make sure we don't recurse infinitely
> when the X in (X & C1) | C2 is a tree of the form (Y * K1) & K2.
> Use mask_with_trailing_zeros where appropriate.
>
>
> 2013-09-09 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
>
> PR tree-optimization/58088
> * gcc.c-torture/compile/pr58088.c: New test.=
>
>
> pr58088.patch
>
>
@@ -9942,6 +9942,22 @@ exact_inverse (tree type, tree cst)
}
}
+/* Mask out the tz least significant bits of X of type TYPE where
+ tz is the number of trailing zeroes in Y. */
+static double_int
+mask_with_tz (tree type, double_int x, double_int y)
+{
+ int tz = y.trailing_zeros ();
+ if (tz > 0)
blank line between declarations and statements.
@@ -11266,6 +11282,7 @@ fold_binary_loc (location_t loc,
{
double_int c1, c2, c3, msk;
int width = TYPE_PRECISION (type), w;
+ bool valid = true;
c1 = tree_to_double_int (TREE_OPERAND (arg0, 1));
c2 = tree_to_double_int (arg1);
blank line after declarations before code body.
}
- if (c3 != c1)
+ /* If X is a tree of the form (Y * K1) & K2, this might conflict
Should be a blank line before the comment as well
+ with that optimization from the BIT_AND_EXPR optimizations.
+ This could end up in an infinite recursion. */
+ if (TREE_CODE (TREE_OPERAND (arg0, 0)) == MULT_EXPR
+ && TREE_CODE (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1))
+ == INTEGER_CST)
+ {
+ tree t = TREE_OPERAND (TREE_OPERAND (arg0, 0), 1);
+ double_int masked = mask_with_tz (type, c3, tree_to_double_int (t));
+ valid = masked != c1;
blank line before statements after declarations.
+ }
+
+ if (c3 != c1 && valid)
'valid' should come before the comparison test. Furthermore, I think
'valid' is misleading; 'try_simplify' would be a more accurate
description of what the test is about.
OK with those changes.
R.
