tree: Improve skip_simple_arithmetic [PR119183]

The following testcase takes very long time to compile, because
skip_simple_arithmetic decides to first call tree_invariant_p on
the second argument (and indirectly recurse there).  I think before
canonicalization of operands for commutative binary expressions
(and for non-commutative ones always) it is pretty common that the
first operand is a constant, something which tree_invariant_p handles
immediately, so the following patch special cases that; I've added
there a tree_invariant_p call too after the checks, while it is not
really needed currently, tree_invariant_p has the same checks, I wanted
to be prepared in case tree_invariant_p changes.  But if you think
I should avoid it, I can drop it too.

This is just a partial fix, I think one can certainly construct a testcase
which will still have horrible compile time complexity (but I've tried and
haven't managed to do so), so perhaps we should just limit the recursion
depth through skip_simple_arithmetic/tree_invariant_p with some defaulted
argument.

2025-03-11  Jakub Jelinek  <jakub@redhat.com>

	PR c/119183
	* tree.cc (skip_simple_arithmetic): If first operand of binary
	expr is TREE_CONSTANT or TREE_READONLY with no side-effects, call
	tree_invariant_p on that operand first instead of on the second.

	* gcc.dg/pr119183.c: New test.

diff --git a/gcc/testsuite/gcc.dg/pr119183.c b/gcc/testsuite/gcc.dg/pr119183.c
new file mode 100644 (file)
index 0000000..a98d779
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr119183.c
@@ -0,0 +1,12 @@
+/* PR c/119183 */
+/* { dg-do compile } */
+
+int foo (void);
+#define A(x) (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (x)))))))))
+
+float
+bar (float r)
+{
+  r += A (A (A (A (A (A (A (A (foo ()))))))));
+  return r;
+}

diff --git a/gcc/tree.cc b/gcc/tree.cc
index a35b4dbe6557ab3e2bd0a0c84f7dc44a3ddef37d..eccfcc89da40b6929752b04f17a9ca3c6eb15468 100644 (file)
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -4105,7 +4105,19 @@ skip_simple_arithmetic (tree expr)
        expr = TREE_OPERAND (expr, 0);
       else if (BINARY_CLASS_P (expr))
        {
-         if (tree_invariant_p (TREE_OPERAND (expr, 1)))
+         /* Before commutative binary operands are canonicalized,
+            it is quite common to have constants in the first operand.
+            Check for that common case first so that we don't walk
+            large expressions with tree_invariant_p unnecessarily.
+            This can still have terrible compile time complexity,
+            we should limit the depth of the tree_invariant_p and
+            skip_simple_arithmetic recursion.  */
+         if ((TREE_CONSTANT (TREE_OPERAND (expr, 0))
+              || (TREE_READONLY (TREE_OPERAND (expr, 0))
+                  && !TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 0))))
+             && tree_invariant_p (TREE_OPERAND (expr, 0)))
+           expr = TREE_OPERAND (expr, 1);
+         else if (tree_invariant_p (TREE_OPERAND (expr, 1)))
            expr = TREE_OPERAND (expr, 0);
          else if (tree_invariant_p (TREE_OPERAND (expr, 0)))
            expr = TREE_OPERAND (expr, 1);