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.

Reply via email to