Re: [PATCH][Resend][tree-optimization] Fix PR58088

2013-09-17 Thread Richard Earnshaw
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.



Re: [PATCH][Resend][tree-optimization] Fix PR58088

2013-09-16 Thread Kyrill Tkachov

Ping.

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.





[PATCH][Resend][tree-optimization] Fix PR58088

2013-09-09 Thread Kyrylo Tkachov
[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
Description: Binary data